Nest the pnacl section of untrusted.gypi to the same level.

Nest the pnacl section of untrusted.gypi to the same level.

The pnacl sections of untrusted.gypi were nested one layer less deep
than other toolchains. This seems to result in >(_sources) being
expanded earlier for pnacl, resulting in misses sources in the
case of base_nacl.gyp in the chrome tree. This is likely because
this is one of the only places that we use target_defaults
to inject a non-variable 'sources' list.

Nesting to the same level seems to alleviate the issue.
(Hopefully giving us more correct dependencies.)

BUG=https://code.google.com/p/chromium/issues/detail?id=427427
TEST=local ninja check + trybots
R=mseaborn@chromium.org, derek@chromium.org

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=14152

[Mark Seaborn, 2014-10-30 18:25:08]

LGTM, though:

 * You'll need to co-ordinate with https://codereview.chromium.org/666693005/

 * I do wonder whether this might introduce subtle dependency bugs too.  Is
there a way we can test for that?

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1285: ['1==1', { # Always true condition to match the
nesting level of the
"True" rather than "1==1"?

Can you add some explanation of why this is necessary for correctness, please,
and add a link to the issue?

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1293: # Flag to translate pexes into nexes at build time
(vs in the browser).
Wrap comments for lines >80 chars?

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1325: # Omitted from non-IRT libraries to keep the
libraries themselves small.
Wrap

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1383: # TODO(dschuff): try removing gline-tables-only after
3.5 merge
Wrap

Same for others below...

[Mark Seaborn, 2014-11-06 05:26:00]

Ping -- are you going to commit this?

[dcheng, 2014-11-25 23:25:06]

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

[dcheng, 2014-11-25 23:25:06]

Any chance this can be landed? This took me 2+ hours to track down, since it
causes https://codereview.chromium.org/760513002/ to break incremental builds,
and is only going to cause more problems in the future.

[bradn, 2014-12-01 19:15:46]

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1285: ['1==1', { # Always true condition to match the
nesting level of the
On 2014/10/30 18:25:07, Mark Seaborn wrote:
> "True" rather than "1==1"?
> 
> Can you add some explanation of why this is necessary for correctness, please,
> and add a link to the issue?

I've expanded the comment.
I'd file an issue but I'm not sure filing a "gyp expansion order is weird" will
produce anything useful.

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1293: # Flag to translate pexes into nexes at build time
(vs in the browser).
On 2014/10/30 18:25:07, Mark Seaborn wrote:
> Wrap comments for lines >80 chars?

Done.

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1325: # Omitted from non-IRT libraries to keep the
libraries themselves small.
On 2014/10/30 18:25:07, Mark Seaborn wrote:
> Wrap

Done.

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1383: # TODO(dschuff): try removing gline-tables-only after
3.5 merge
On 2014/10/30 18:25:07, Mark Seaborn wrote:
> Wrap
> 
> Same for others below...

Done.

[Mark Seaborn, 2014-12-01 19:18:47]

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1285: ['1==1', { # Always true condition to match the
nesting level of the
On 2014/12/01 19:15:46, bradn wrote:
> On 2014/10/30 18:25:07, Mark Seaborn wrote:
> > "True" rather than "1==1"?
> > 
> > Can you add some explanation of why this is necessary for correctness,
please,
> > and add a link to the issue?
> 
> I've expanded the comment.
> I'd file an issue but I'm not sure filing a "gyp expansion order is weird"
will
> produce anything useful.

I meant link to the issue you're fixing:
https://code.google.com/p/chromium/issues/detail?id=427427

[bradn, 2014-12-01 21:39:23]

Committed patchset #5 (id:80001) manually as 14152 (presubmit successful).

[Mark Seaborn, 2014-12-04 22:03:19]

This apparently broke an MSAN bot after rolling into Chromium.

See: https://codereview.chromium.org/774883002/#msg19

Please can you investigate and/or revert?

[bradn, 2014-12-04 22:09:28]

Forgot to send these out.
Looking into if I should revert.

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/684033002/diff/1/build/untrusted.gypi#newcode...
build/untrusted.gypi:1285: ['1==1', { # Always true condition to match the
nesting level of the
On 2014/12/01 19:18:46, Mark Seaborn wrote:
> On 2014/12/01 19:15:46, bradn wrote:
> > On 2014/10/30 18:25:07, Mark Seaborn wrote:
> > > "True" rather than "1==1"?
> > > 
> > > Can you add some explanation of why this is necessary for correctness,
> please,
> > > and add a link to the issue?
> > 
> > I've expanded the comment.
> > I'd file an issue but I'm not sure filing a "gyp expansion order is weird"
> will
> > produce anything useful.
> 
> I meant link to the issue you're fixing:
> https://code.google.com/p/chromium/issues/detail?id=427427

Ah, ok. done.