Adds -Wno-deprecated-register to the build settings for iOS.

Adds -Wno-deprecated-register to the build settings for iOS.

Xcode 5.1 ships with a clang that deprecates the register keyword by default.
This warning flag is needed in order to compile for iOS with Xcode 5.1.

However, the old version of Xcode (5.0) does not understand this flag.  In order
to temporarily support compiling on both versions of Xcode, this CL also adds
-Wno-unknown-warning-option to suppress errors about unknown compiler flags.
This flag will be removed as soon as all of the bots have been updated with
Xcode 5.1, within a week.

BUG=255186, 337611
TEST=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256592

[rohitrao (ping after 24h), 2014-03-11 20:43:24]

Sending this out to see if anyone can explain why my first attempt at putting a
conditions block inside the ios block doesn't work.

The condition is there to try and skip adding the flag when compiling with
Chromium's clang.

If no one has an answer, how does this alternative approach look to everyone? 
I'd like to gets the bots updated and remove -Wno-unknown-warning-option by the
end of next week, preferably even by the end of this week.

https://codereview.chromium.org/194803006/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/194803006/diff/20001/build/common.gypi#newcod...
build/common.gypi:4199: ['OS=="ios" and ("<(GENERATOR)"=="xcode" or
clang_xcode==1)', {
This conditions clause works if I put it here, but not if I add a conditions
block inside xcode_settings in the ios block below (eg at line 4477).  I don't
understand why.

https://codereview.chromium.org/194803006/diff/20001/build/common.gypi#newcod...
build/common.gypi:4477: 
If I put the following code here, the flag gets ignored completely:

          'conditions': [
            ['"<(GENERATOR)"=="xcode" or clang_xcode==1', {
              'WARNING_CFLAGS': [
                '-Wno-unknown-warning-option',
              ],
            }],
          ],

[Mark Mentovai, 2014-03-11 20:46:37]

https://codereview.chromium.org/194803006/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/194803006/diff/20001/build/common.gypi#newcod...
build/common.gypi:4477: 
rohitrao wrote:
> If I put the following code here, the flag gets ignored completely:
> 
>           'conditions': [
>             ['"<(GENERATOR)"=="xcode" or clang_xcode==1', {
>               'WARNING_CFLAGS': [
>                 '-Wno-unknown-warning-option',
>               ],
>             }],
>           ],

There is a conditions block right below this line already, and it will clobber
this one.

Write the condition inside of the existing conditions block.

[Nico, 2014-03-11 20:48:05]

BUG=255186,337611


On Tue, Mar 11, 2014 at 1:46 PM, <mark@chromium.org> wrote:

>
> https://codereview.chromium.org/194803006/diff/20001/build/common.gypi
> File build/common.gypi (right):
>
> https://codereview.chromium.org/194803006/diff/20001/
> build/common.gypi#newcode4477
> build/common.gypi:4477:
>
> rohitrao wrote:
>
>> If I put the following code here, the flag gets ignored completely:
>>
>
>             'conditions': [
>>              ['"<(GENERATOR)"=="xcode" or clang_xcode==1', {
>>                'WARNING_CFLAGS': [
>>                  '-Wno-unknown-warning-option',
>>                ],
>>              }],
>>            ],
>>
>
> There is a conditions block right below this line already, and it will
> clobber this one.
>
> Write the condition inside of the existing conditions block.
>
> https://codereview.chromium.org/194803006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[rohitrao (ping after 24h), 2014-03-11 21:09:39]

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi#newcod...
build/common.gypi:4468: '-Wno-deprecated-register',
-Wno-deprecated-register is now passed *twice* for iOS ninja builds.  It's added
once in the "mac or ios, clang==1" section above, and a second time here.

Is this a big deal?  The alternative is to conditionally add it in the iOS block
if ("<(GENERATOR)"=="xcode").

(Do we still support building Mac with Xcode?  Could we just unconditionally add
-Wno-deprecated-register and break Xcode builds, or would that make people mad?

[Nico, 2014-03-11 21:17:46]

On Tue, Mar 11, 2014 at 2:09 PM, <rohitrao@chromium.org> wrote:

>
> https://codereview.chromium.org/194803006/diff/30001/build/common.gypi
> File build/common.gypi (right):
>
> https://codereview.chromium.org/194803006/diff/30001/
> build/common.gypi#newcode4468
> build/common.gypi:4468: '-Wno-deprecated-register',
> -Wno-deprecated-register is now passed *twice* for iOS ninja builds.
> It's added once in the "mac or ios, clang==1" section above, and a
> second time here.
>
> Is this a big deal?  The alternative is to conditionally add it in the
> iOS block if ("<(GENERATOR)"=="xcode").
>
> (Do we still support building Mac with Xcode?


no


>  Could we just
> unconditionally add -Wno-deprecated-register and break Xcode builds, or
> would that make people mad?
>
> https://codereview.chromium.org/194803006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mark Mentovai, 2014-03-11 21:39:51]

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi#newcod...
build/common.gypi:4468: '-Wno-deprecated-register',
rohitrao wrote:
> -Wno-deprecated-register is now passed *twice* for iOS ninja builds.  It's
added
> once in the "mac or ios, clang==1" section above, and a second time here.

I’m taking that to mean that you’re not always using clang? Really?

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi#newcod...
build/common.gypi:4471: # Limit the valid architectures depending on
"target_subarch".
This comment goes with the target_subarch conditions that were already there,
but now you’ve separated them.

[rohitrao (ping after 24h), 2014-03-11 21:49:31]

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi#newcod...
build/common.gypi:4468: '-Wno-deprecated-register',
> I’m taking that to mean that you’re not always using clang? Really?

"clang==1" is only true if you're using Ninja.  It's false for Xcode builds. 
Most developers are on ninja by now, but the bots (and official builders) are
still using Xcode.

Xcode itself is using clang too, but that doesn't mean "clang==1" =)

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi#newcod...
build/common.gypi:4471: # Limit the valid architectures depending on
"target_subarch".
On 2014/03/11 21:39:52, Mark Mentovai wrote:
> This comment goes with the target_subarch conditions that were already there,
> but now you’ve separated them.

Done.

[sdefresne, 2014-03-11 21:50:56]

I've got the follow change to re-enable the -Wno-deprecated-register flag for
iOS when using ninja: https://codereview.chromium.org/195793003

It will break the ninja fat fyi bot, but I think it should be combined with your
change. I think it would be enough to move the '-Wno-deprecated-register'
outside of the 'xcode==1' condition, and then the flag will not be set twice.

Since I think I'm the only one building fat binaries, and I've already updated
to Xcode 5.1, this won't break anyone workflow.

[Mark Mentovai, 2014-03-11 21:55:46]

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/194803006/diff/30001/build/common.gypi#newcod...
build/common.gypi:4468: '-Wno-deprecated-register',
rohitrao wrote:
> > I’m taking that to mean that you’re not always using clang? Really?
> 
> "clang==1" is only true if you're using Ninja.  It's false for Xcode builds. 

That sounds wrongish to me.

If you’re using clang everywhere, can you set clang=1 even in your Xcode builds?

[Nico, 2014-03-11 21:59:43]

On Tue, Mar 11, 2014 at 2:49 PM, <rohitrao@chromium.org> wrote:

>
> https://codereview.chromium.org/194803006/diff/30001/build/common.gypi
> File build/common.gypi (right):
>
> https://codereview.chromium.org/194803006/diff/30001/
> build/common.gypi#newcode4468
> build/common.gypi:4468: '-Wno-deprecated-register',
>
>> I'm taking that to mean that you're not always using clang? Really?
>>
>
> "clang==1" is only true if you're using Ninja.  It's false for Xcode
> builds.  Most developers are on ninja by now, but the bots (and official
> builders) are still using Xcode.
>
> Xcode itself is using clang too, but that doesn't mean "clang==1" =)


I thought https://src.chromium.org/viewvc/chrome?view=rev&revision=251083 fixed
this?


>
>
> https://codereview.chromium.org/194803006/diff/30001/
> build/common.gypi#newcode4471
> build/common.gypi:4471: # Limit the valid architectures depending on
> "target_subarch".
> On 2014/03/11 21:39:52, Mark Mentovai wrote:
>
>> This comment goes with the target_subarch conditions that were already
>>
> there,
>
>> but now you've separated them.
>>
>
> Done.
>
> https://codereview.chromium.org/194803006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[rohitrao (ping after 24h), 2014-03-11 22:13:51]

Ah, Sylvain's change will probably let us clean this all up.  So we'll have
three worlds:

1) Compiling using Xcode (which implies Xcode's clang):
GYP_GENERATOR=xcode
clang == 1
clang_xcode == 1

2) Compiling with Ninja, using upstream's clang:
GYP_GENERATOR=ninja
clang == 1
clang_xcode == 0

3) Compiling with Ninja, using Xcode's clang:
GYP_GENERATOR=ninja
clang == 1
clang_xcode == 1


We can rewrite common.gypi to set compiler flags based on combinations of those
three variables, rather than the crap we have now.  Does everyone like that
plan?


I do think that rewriting common.gypi is independent of this CL to get Xcode 5.1
working, though :-D

[stuartmorgan, 2014-03-11 22:41:34]

On 2014/03/11 22:13:51, rohitrao wrote:
> We can rewrite common.gypi to set compiler flags based on combinations of
those
> three variables, rather than the crap we have now.  Does everyone like that
> plan?

Very much :) I think that structure came partially out of a conversation I had
with Nico about my sadness about the current situation, so I'm glad to see that
happening.

[Nico, 2014-03-11 22:46:02]

On Tue, Mar 11, 2014 at 3:41 PM, <stuartmorgan@chromium.org> wrote:

> On 2014/03/11 22:13:51, rohitrao wrote:
>
>> We can rewrite common.gypi to set compiler flags based on combinations of
>>
> those
>
>> three variables, rather than the crap we have now.  Does everyone like
>> that
>> plan?
>>
>
> Very much :) I think that structure came partially out of a conversation I
> had
> with Nico about my sadness about the current situation, so I'm glad to see
> that
> happening.
>

Yup, +1 from me too.


>
> https://codereview.chromium.org/194803006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sdefresne, 2014-03-12 08:31:46]

+1 from me too.

[rohitrao (ping after 24h), 2014-03-12 15:16:35]

I will work on a common.gypi cleanup today.

In the meantime, can I proceed with this CL as written, so that infra can start
converting the iOS bots?

[Mark Mentovai, 2014-03-12 15:32:41]

Who are you asking, me? Sure. LGTM.

[sdefresne, 2014-03-12 15:33:21]

LGTM for me too.

[rohitrao (ping after 24h), 2014-03-12 15:49:07]

The CQ bit was checked by rohitrao@chromium.org

[stuartmorgan, 2014-03-12 15:49:16]

lgtm

[commit-bot: I haz the power, 2014-03-12 15:50:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rohitrao@chromium.org/194803006/70001

[commit-bot: I haz the power, 2014-03-12 16:59:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rohitrao@chromium.org/194803006/70001

[commit-bot: I haz the power, 2014-03-12 19:07:00]

Change committed as 256592