Fix imports in gcc_toolchain.gni to not rely on //tools.

Fix imports in gcc_toolchain.gni to not rely on //tools.

Importing //tools into gcc_toolchain.gni breaks certain downstream
projects that include //build but not //tools. This CL changes
gcc_toolchain.gni to only rely on files within //build.

BUG=641981
Committed: https://crrev.com/47f910a6e58b633bd902adce24feecf26901f84d
Cr-Commit-Position: refs/heads/master@{#415113}

[estevenson, 2016-08-29 16:09:00]

Description was changed from

==========
Fix imports in gcc_toolchain.gni to not rely on //tools.

Importing //tools into gcc_toolchain.gni breaks certain downstream
projects that include //build but not //tools. This CL changes
gcc_toolchain.gni to only rely on files within //build.

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

to

==========
Fix imports in gcc_toolchain.gni to not rely on //tools.

Importing //tools into gcc_toolchain.gni breaks certain downstream
projects that include //build but not //tools. This CL changes
gcc_toolchain.gni to only rely on files within //build.

BUG=641981
==========

[estevenson, 2016-08-29 16:19:33]

estevenson@chromium.org changed reviewers:
+ dpranke@chromium.org

[estevenson, 2016-08-29 16:19:33]

Hi Dirk, ptal!

[Dirk Pranke, 2016-08-29 16:33:00]

lgtm w/ suggested change.

https://codereview.chromium.org/2288273002/diff/20001/build/toolchain/gcc_too...
File build/toolchain/gcc_toolchain.gni (right):

https://codereview.chromium.org/2288273002/diff/20001/build/toolchain/gcc_too...
build/toolchain/gcc_toolchain.gni:223: enable_resource_whitelist_generation
This should just import("//build/config/android/config.gni") and just reference
enable_resource_whitelist_generation directly as you were before. You could
probably also wrap that in an `if (is_android)` block to avoid the otherwise
unnecessary import.

[estevenson, 2016-08-29 17:41:13]

Ptal Dirk!

https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
File build/toolchain/gcc_toolchain.gni (right):

https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
build/toolchain/gcc_toolchain.gni:5: import("//build/config/android/config.gni")
Hiding this behind is_android would require an
is_defined(enable_resource_whitelist_generation) check, would that be
preferable? Everything besides setting enable_resource_whitelist_generation is
already behind is_android already.

[Dirk Pranke, 2016-08-29 18:15:11]

https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
File build/toolchain/gcc_toolchain.gni (right):

https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
build/toolchain/gcc_toolchain.gni:5: import("//build/config/android/config.gni")
On 2016/08/29 17:41:12, estevenson wrote:
> Hiding this behind is_android would require an
> is_defined(enable_resource_whitelist_generation) check, would that be
> preferable? Everything besides setting enable_resource_whitelist_generation is
> already behind is_android already.

is_android is a global variable, so why would that require an
is_defined(enable_resource_whitelist_generation) check?

Alternatively, if enable_resource_whitelist_generation is an android-only
concept, we could make this a toolchain arg.

At any rate, we can also land and iterate, to unbreak things.

[Michael Achenbach, 2016-08-29 18:15:29]

machenbach@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Achenbach, 2016-08-29 18:15:30]

lgtm, thanks for fixing!

[estevenson, 2016-08-29 18:20:10]

The CQ bit was checked by estevenson@chromium.org

[estevenson, 2016-08-29 18:20:11]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2288273002/#ps40001
(title: "Address dpranke comments")

[estevenson, 2016-08-29 18:22:12]

On 2016/08/29 18:15:11, Dirk Pranke wrote:
>
https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
> File build/toolchain/gcc_toolchain.gni (right):
> 
>
https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
> build/toolchain/gcc_toolchain.gni:5:
import("//build/config/android/config.gni")
> On 2016/08/29 17:41:12, estevenson wrote:
> > Hiding this behind is_android would require an
> > is_defined(enable_resource_whitelist_generation) check, would that be
> > preferable? Everything besides setting enable_resource_whitelist_generation
is
> > already behind is_android already.
> 
> is_android is a global variable, so why would that require an
> is_defined(enable_resource_whitelist_generation) check?
> 
> Alternatively, if enable_resource_whitelist_generation is an android-only
> concept, we could make this a toolchain arg.
> 
> At any rate, we can also land and iterate, to unbreak things.

The toolchain arg might be a better idea. Landing this for now and will see if I
can come up with a cleaner fix.

[estevenson, 2016-08-29 18:22:56]

On 2016/08/29 18:22:12, estevenson wrote:
> On 2016/08/29 18:15:11, Dirk Pranke wrote:
> >
>
https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
> > File build/toolchain/gcc_toolchain.gni (right):
> > 
> >
>
https://codereview.chromium.org/2288273002/diff/40001/build/toolchain/gcc_too...
> > build/toolchain/gcc_toolchain.gni:5:
> import("//build/config/android/config.gni")
> > On 2016/08/29 17:41:12, estevenson wrote:
> > > Hiding this behind is_android would require an
> > > is_defined(enable_resource_whitelist_generation) check, would that be
> > > preferable? Everything besides setting
enable_resource_whitelist_generation
> is
> > > already behind is_android already.
> > 
> > is_android is a global variable, so why would that require an
> > is_defined(enable_resource_whitelist_generation) check?
> > 
> > Alternatively, if enable_resource_whitelist_generation is an android-only
> > concept, we could make this a toolchain arg.
> > 
> > At any rate, we can also land and iterate, to unbreak things.
> 
> The toolchain arg might be a better idea. Landing this for now and will see if
I
> can come up with a cleaner fix.

Thanks for the quick review!

[commit-bot: I haz the power, 2016-08-29 18:27:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[estevenson, 2016-08-29 18:58:41]

The CQ bit was checked by estevenson@chromium.org to run a CQ dry run

[Dirk Pranke, 2016-08-29 19:10:04]

still lgtm.

[commit-bot: I haz the power, 2016-08-29 19:17:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[estevenson, 2016-08-29 22:36:42]

The CQ bit was unchecked by estevenson@chromium.org

[estevenson, 2016-08-29 22:37:28]

The CQ bit was checked by estevenson@chromium.org

[estevenson, 2016-08-29 22:37:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
machenbach@chromium.org
Link to the patchset: https://codereview.chromium.org/2288273002/#ps60001
(title: "Add missing import to grit_rule.gni")

[commit-bot: I haz the power, 2016-08-30 00:01:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 04:46:30]

Description was changed from

==========
Fix imports in gcc_toolchain.gni to not rely on //tools.

Importing //tools into gcc_toolchain.gni breaks certain downstream
projects that include //build but not //tools. This CL changes
gcc_toolchain.gni to only rely on files within //build.

BUG=641981
==========

to

==========
Fix imports in gcc_toolchain.gni to not rely on //tools.

Importing //tools into gcc_toolchain.gni breaks certain downstream
projects that include //build but not //tools. This CL changes
gcc_toolchain.gni to only rely on files within //build.

BUG=641981
==========

[commit-bot: I haz the power, 2016-08-30 04:46:31]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-30 04:48:33]

Description was changed from

==========
Fix imports in gcc_toolchain.gni to not rely on //tools.

Importing //tools into gcc_toolchain.gni breaks certain downstream
projects that include //build but not //tools. This CL changes
gcc_toolchain.gni to only rely on files within //build.

BUG=641981
==========

to

==========
Fix imports in gcc_toolchain.gni to not rely on //tools.

Importing //tools into gcc_toolchain.gni breaks certain downstream
projects that include //build but not //tools. This CL changes
gcc_toolchain.gni to only rely on files within //build.

BUG=641981

Committed: https://crrev.com/47f910a6e58b633bd902adce24feecf26901f84d
Cr-Commit-Position: refs/heads/master@{#415113}
==========

[commit-bot: I haz the power, 2016-08-30 04:48:34]

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