Fix build with disabled NaCl glibc toolchain.

Fix build with disabled NaCl glibc toolchain.

This is upstreaming Gentoo Linux patch.

BUG=none
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=143364

[Paweł Hajdan Jr., 2012-06-12 14:27:22]

Please review:

viettrungluu (OWNERS)
bradchen: FYI

[Brad Chen, 2012-06-12 16:50:16]

Once the bots are green I will want to see that the glibc nacl tests are still
running on the Chromium bots.

I'm sorry that cross compilation under gyp is a little bit convoluted right now.
If you want help figuring this out let me know.

On 2012/06/12 14:27:22, Paweł Hajdan Jr. wrote:
> Please review:
> 
> viettrungluu (OWNERS)
> bradchen: FYI

[Paweł Hajdan Jr., 2012-06-12 20:07:17]

On 2012/06/12 16:50:16, Brad Chen wrote:
> Once the bots are green I will want to see that the glibc nacl tests are still
> running on the Chromium bots.

Yeah, this redness is bad. I tested locally against latest dev channel release.
Maybe the trunk has gone further. :-/

> I'm sorry that cross compilation under gyp is a little bit convoluted right
now.
> If you want help figuring this out let me know.

Note: this is not about cross compilation. It's more like: delete
native_client/toolchain/linux_x86 directory, use -Ddisable_glibc=1 gyp flag, and
the build of chrome should succeed.

> On 2012/06/12 14:27:22, Paweł Hajdan Jr. wrote:
> > Please review:
> > 
> > viettrungluu (OWNERS)
> > bradchen: FYI

[viettrungluu, 2012-06-12 20:18:37]

I know almost nothing about this, so if/when it's good enough for Brad, it'll be
good enough for me.

[Brad Chen (chromium), 2012-06-12 20:29:34]

Let me know when you'd like me to have another look.

On Tue, Jun 12, 2012 at 1:18 PM, <viettrungluu@chromium.org> wrote:

> I know almost nothing about this, so if/when it's good enough for Brad,
> it'll be
> good enough for me.
>
>
https://chromiumcodereview.**appspot.com/10537124/<https://chromiumcodereview...
>

[Paweł Hajdan Jr., 2012-06-20 09:22:11]

PTAL

[Brad Chen, 2012-06-20 19:09:07]

LGTM

[commit-bot: I haz the power, 2012-06-21 06:28:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/phajdan.jr@chromium.org/10537124/17001

[commit-bot: I haz the power, 2012-06-21 06:29:02]

Presubmit check for 10537124-17001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    ppapi

[Paweł Hajdan Jr., 2012-06-21 10:29:53]

This change has now landed.

Brad, could you please remove definitions of gyp variable disable_glibc from
src/native_client/build/untrusted.gypi? I don't think I have access to NaCl
repo, and those definitions are now redundant (and they should be removed, to
avoid possible mismatch).

[Brad Chen, 2012-06-21 16:13:37]

I don't think we can do this as it would break the NaCl gyp build. If there is a
bigger issue besides just the hygiene question we should consult with Brad
Nelson as he has a better global grasp of gyp than I do.

Brad

On 2012/06/21 10:29:53, Paweł Hajdan Jr. wrote:
> This change has now landed.
> 
> Brad, could you please remove definitions of gyp variable disable_glibc from
> src/native_client/build/untrusted.gypi? I don't think I have access to NaCl
> repo, and those definitions are now redundant (and they should be removed, to
> avoid possible mismatch).

[Paweł Hajdan Jr., 2012-06-21 17:16:46]

+bradnelson

On 2012/06/21 16:13:37, Brad Chen wrote:
> I don't think we can do this as it would break the NaCl gyp build. If there is
a
> bigger issue besides just the hygiene question we should consult with Brad
> Nelson as he has a better global grasp of gyp than I do.

Are you sure that would be a breaking change? My understanding of gyp is that
the definition from build/common.gypi will be used.

I think it's more than a cosmetic issue - messy gyp files easily lead to other
problems, and nacl ones are already complicated.

[bradn, 2012-06-21 17:51:22]

So, why was this causing being in untrusted.gypi? It's got % after it and is
used in ppapi_untrusted.
Also its a shame this got named something as ambiguous as disable_glibc (that's
kinda of broad, probably should have been disable_nacl_glibc or something).

[Paweł Hajdan Jr., 2012-06-21 18:03:00]

On 2012/06/21 17:51:22, bradn wrote:
> So, why was this causing being in untrusted.gypi? It's got % after it and is
> used in ppapi_untrusted.

I don't understand the question. Patch Set 1 fixes a problem that when glibc
toolchain is missing (Linux distros shouldn't need it), without that condition
check for disable_glibc==0 the action below would fail without glibc toolchain.
Patch Set 2 fixes a problem with disable_glibc not being visible in
ppapi_untrusted.gyp (despite untrusted.gypi being included, I don't understand
that). See
http://build.chromium.org/p/tryserver.chromium/builders/linux_rel/builds/3389...

> Also its a shame this got named something as ambiguous as disable_glibc
(that's
> kinda of broad, probably should have been disable_nacl_glibc or something).

Feel free to rename that. I agree with that conclusion.

[Brad Chen, 2012-06-22 17:50:33]

This is a breaking change in NaCl because NaCl gyp does not depend on
Chrome. We are trying to eliminate the Chrome dependency in NaCl to
eliminate a circular dependency.

Maybe I need to figure out a change to native_client/build/common.gypi to
complement your change in Chrome; I'll ask Brad about that.

Cross compilation in GYP is generally a mess. Brad has some good ideas
about making things better.

Brad

On Thu, Jun 21, 2012 at 10:16 AM, <phajdan.jr@chromium.org> wrote:

> +bradnelson
>
>
> On 2012/06/21 16:13:37, Brad Chen wrote:
>
>> I don't think we can do this as it would break the NaCl gyp build. If
>> there is
>>
> a
>
>> bigger issue besides just the hygiene question we should consult with Brad
>> Nelson as he has a better global grasp of gyp than I do.
>>
>
> Are you sure that would be a breaking change? My understanding of gyp is
> that
> the definition from build/common.gypi will be used.
>
> I think it's more than a cosmetic issue - messy gyp files easily lead to
> other
> problems, and nacl ones are already complicated.
>
>
http://codereview.chromium.**org/10537124/<http://codereview.chromium.org/105...
>

[bradn, 2012-06-23 01:02:20]

Ok, I've got a fix ready that will still gate this from untrusted.gypi, but
allow the condition you added to ppapi.
I need to run, but I'll send it out on monday.

-BradN


On Fri, Jun 22, 2012 at 10:50 AM, Brad Chen <bradchen@google.com> wrote:

> This is a breaking change in NaCl because NaCl gyp does not depend on
> Chrome. We are trying to eliminate the Chrome dependency in NaCl to
> eliminate a circular dependency.
>
> Maybe I need to figure out a change to native_client/build/common.gypi to
> complement your change in Chrome; I'll ask Brad about that.
>
> Cross compilation in GYP is generally a mess. Brad has some good ideas
> about making things better.
>
> Brad
>
>
> On Thu, Jun 21, 2012 at 10:16 AM, <phajdan.jr@chromium.org> wrote:
>
>> +bradnelson
>>
>>
>> On 2012/06/21 16:13:37, Brad Chen wrote:
>>
>>> I don't think we can do this as it would break the NaCl gyp build. If
>>> there is
>>>
>> a
>>
>>> bigger issue besides just the hygiene question we should consult with
>>> Brad
>>> Nelson as he has a better global grasp of gyp than I do.
>>>
>>
>> Are you sure that would be a breaking change? My understanding of gyp is
>> that
>> the definition from build/common.gypi will be used.
>>
>> I think it's more than a cosmetic issue - messy gyp files easily lead to
>> other
>> problems, and nacl ones are already complicated.
>>
>>
http://codereview.chromium.**org/10537124/<http://codereview.chromium.org/105...
>>
>
>