Get iOS GN build working again

Get iOS GN build working again.

This CL adds some files and compile flags that were missing in the GN build. This is a prelude to adding the GN iOS bots to the main waterfall and CQ.

R=sdefresne@chromium.org, brettw@chromium.org
BUG=459705
Committed: https://crrev.com/34d6652e39479abf69f9676257e393becd93dbe5
Cr-Commit-Position: refs/heads/master@{#376584}

[Dirk Pranke, 2016-02-19 05:44:59]

Description was changed from

==========
dummy change to force MB to clobber build dirs

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

to

==========
Get iOS GN build working again.

This CL adds some files and compile flags that were missing in the GN build.
This is a prelude to adding the GN iOS bots to the main waterfall and CQ.

R=sdefresne@chromium.org, brettw@chromium.org
BUG=459705
==========

[Dirk Pranke, 2016-02-19 05:44:59]

dpranke@chromium.org changed reviewers:
+ brettw@chromium.org, sdefresne@chromium.org

[brettw, 2016-02-19 23:30:55]

lgtm

[Dirk Pranke, 2016-02-19 23:33:03]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2016-02-19 23:37:15]

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

[commit-bot: I haz the power, 2016-02-20 00:28:26]

Description was changed from

==========
Get iOS GN build working again.

This CL adds some files and compile flags that were missing in the GN build.
This is a prelude to adding the GN iOS bots to the main waterfall and CQ.

R=sdefresne@chromium.org, brettw@chromium.org
BUG=459705
==========

to

==========
Get iOS GN build working again.

This CL adds some files and compile flags that were missing in the GN build.
This is a prelude to adding the GN iOS bots to the main waterfall and CQ.

R=sdefresne@chromium.org, brettw@chromium.org
BUG=459705
==========

[commit-bot: I haz the power, 2016-02-20 00:28:27]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-20 00:30:05]

Description was changed from

==========
Get iOS GN build working again.

This CL adds some files and compile flags that were missing in the GN build.
This is a prelude to adding the GN iOS bots to the main waterfall and CQ.

R=sdefresne@chromium.org, brettw@chromium.org
BUG=459705
==========

to

==========
Get iOS GN build working again.

This CL adds some files and compile flags that were missing in the GN build.
This is a prelude to adding the GN iOS bots to the main waterfall and CQ.

R=sdefresne@chromium.org, brettw@chromium.org
BUG=459705

Committed: https://crrev.com/34d6652e39479abf69f9676257e393becd93dbe5
Cr-Commit-Position: refs/heads/master@{#376584}
==========

[commit-bot: I haz the power, 2016-02-20 00:30:06]

Patchset 5 (id:??) landed as
https://crrev.com/34d6652e39479abf69f9676257e393becd93dbe5
Cr-Commit-Position: refs/heads/master@{#376584}

[sdefresne, 2016-02-22 08:29:29]

lgtm

https://codereview.chromium.org/1698963004/diff/80001/ios/web/BUILD.gn
File ios/web/BUILD.gn (right):

https://codereview.chromium.org/1698963004/diff/80001/ios/web/BUILD.gn#newcod...
ios/web/BUILD.gn:346: "public/test/scoped_Testing_web_client.h",
nit: the files are named scoped_testing_web_client.{h,mm} (i.e. with a
lower-case 't'). While bots and most of the devs use a case-insensitive file
system (this is the default on HFS+) other do use a case-sensitive one to work
on Chrome. Could you fix in a followup?

[Dirk Pranke, 2016-02-22 16:56:00]

https://codereview.chromium.org/1698963004/diff/80001/ios/web/BUILD.gn
File ios/web/BUILD.gn (right):

https://codereview.chromium.org/1698963004/diff/80001/ios/web/BUILD.gn#newcod...
ios/web/BUILD.gn:346: "public/test/scoped_Testing_web_client.h",
On 2016/02/22 08:29:29, sdefresne wrote:
> nit: the files are named scoped_testing_web_client.{h,mm} (i.e. with a
> lower-case 't'). While bots and most of the devs use a case-insensitive file
> system (this is the default on HFS+) other do use a case-sensitive one to work
> on Chrome. Could you fix in a followup?

Whoops. Yes of course, that must've been a typo that snuck in on my part :(.

[Nico, 2016-06-22 22:53:32]

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

[Nico, 2016-06-22 22:53:33]

https://codereview.chromium.org/1698963004/diff/80001/third_party/google_tool...
File third_party/google_toolbox_for_mac/BUILD.gn (right):

https://codereview.chromium.org/1698963004/diff/80001/third_party/google_tool...
third_party/google_toolbox_for_mac/BUILD.gn:17: cflags = [
"-Wno-deprecated-declarations" ]
so this config is a public_config below and this has disabled deprecation
warnings for all code depending on this (e.g. the chrome target).

1. Please fix this
2. How can we make it harder for people to make catastrophic mistakes like this?
Should configs listed in public_configs have to be declared via public_config(),
so that it's visible at the config site that changing it isn't some local thing?

[Dirk Pranke, 2016-06-22 23:34:05]

On 2016/06/22 22:53:33, Nico wrote:
>
https://codereview.chromium.org/1698963004/diff/80001/third_party/google_tool...
> File third_party/google_toolbox_for_mac/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1698963004/diff/80001/third_party/google_tool...
> third_party/google_toolbox_for_mac/BUILD.gn:17: cflags = [
> "-Wno-deprecated-declarations" ]
> so this config is a public_config below and this has disabled deprecation
> warnings for all code depending on this (e.g. the chrome target).
> 
> 1. Please fix this
> 2. How can we make it harder for people to make catastrophic mistakes like
this?
> Should configs listed in public_configs have to be declared via
public_config(),
> so that it's visible at the config site that changing it isn't some local
thing?

"catastrophic" seems like an exaggeration here. 

Has this actually resulted in bugs or more uses of deprecated APIs being
introduced?

I don't know that your suggestion would make things much better or have actually

prevented a change like this. It's not obvious to me that there is something
that we
could do that would be effective and also not actually cause more harm
(overhead)
than good.

[Nico, 2016-06-22 23:41:29]

Well no, we still have gyp bots. But once they're gone, if I hadn't
accidentally noticed this, this would've majorly screwed us over when
upping the deployment target. I don't think catastrophic is an exaggeration.
On Jun 22, 2016 7:34 PM, <dpranke@chromium.org> wrote:

> On 2016/06/22 22:53:33, Nico wrote:
> >
>
>
https://codereview.chromium.org/1698963004/diff/80001/third_party/google_tool...
> > File third_party/google_toolbox_for_mac/BUILD.gn (right):
> >
> >
>
>
https://codereview.chromium.org/1698963004/diff/80001/third_party/google_tool...
> > third_party/google_toolbox_for_mac/BUILD.gn:17: cflags = [
> > "-Wno-deprecated-declarations" ]
> > so this config is a public_config below and this has disabled deprecation
> > warnings for all code depending on this (e.g. the chrome target).
> >
> > 1. Please fix this
> > 2. How can we make it harder for people to make catastrophic mistakes
> like
> this?
> > Should configs listed in public_configs have to be declared via
> public_config(),
> > so that it's visible at the config site that changing it isn't some local
> thing?
>
> "catastrophic" seems like an exaggeration here.
>
> Has this actually resulted in bugs or more uses of deprecated APIs being
> introduced?
>
> I don't know that your suggestion would make things much better or have
> actually
>
> prevented a change like this. It's not obvious to me that there is
> something
> that we
> could do that would be effective and also not actually cause more harm
> (overhead)
> than good.
>
> https://codereview.chromium.org/1698963004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.