Setting is_component_build to false by default

Setting is_component_build to false by default

Webrtc does not support component builds so we want to override
the chromium default value (which can be true on debug builds if the
os is different from iOS).

Please note that the user can set this value to true in two ways:

 - using --args (e.g.: gn gen out/default --args='is_component_build=true'
 - changing the value in the args.gn file

But in both cases the value will be ignored because we don't use the
'component' template but we rely directly on 'rtc_static_library' and
'rtc_shared_library'.

BUG=webrtc:6975
NOTRY=True

Review-Url: https://codereview.webrtc.org/2728643003
Cr-Commit-Position: refs/heads/master@{#17020}
Committed: https://chromium.googlesource.com/external/webrtc/+/2cb3944ba7e4467601ebd94888e1811a6d77be49

[mbonadei, 2017-03-01 17:39:26]

mbonadei@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[mbonadei, 2017-03-01 17:39:27]

Using default_args = {} works. :)

[mbonadei, 2017-03-01 18:20:40]

On 2017/03/01 17:39:27, mbonadei wrote:
> Using default_args = {} works. :)

Let me add an assertion as suggested by dprake@ on the gn-dev mailing list (you
also suggested the same thing yesterday evening).

[mbonadei, 2017-03-01 18:31:54]

On 2017/03/01 18:20:40, mbonadei wrote:
> On 2017/03/01 17:39:27, mbonadei wrote:
> > Using default_args = {} works. :)
> 
> Let me add an assertion as suggested by dprake@ on the gn-dev mailing list
(you
> also suggested the same thing yesterday evening).

PTAL.

I am not 100% about the indentation of the assert error message but it has been
done that way by the 'gn format' command.

[mbonadei, 2017-03-01 18:54:09]

On 2017/03/01 18:31:54, mbonadei wrote:
> On 2017/03/01 18:20:40, mbonadei wrote:
> > On 2017/03/01 17:39:27, mbonadei wrote:
> > > Using default_args = {} works. :)
> > 
> > Let me add an assertion as suggested by dprake@ on the gn-dev mailing list
> (you
> > also suggested the same thing yesterday evening).
> 
> PTAL.
> 
> I am not 100% about the indentation of the assert error message but it has
been
> done that way by the 'gn format' command.

We have to update our recipes I guess. :)

[kjellander_webrtc, 2017-03-02 02:32:15]

On 2017/03/01 18:54:09, mbonadei wrote:
> On 2017/03/01 18:31:54, mbonadei wrote:
> > On 2017/03/01 18:20:40, mbonadei wrote:
> > > On 2017/03/01 17:39:27, mbonadei wrote:
> > > > Using default_args = {} works. :)
> > > 
> > > Let me add an assertion as suggested by dprake@ on the gn-dev mailing list
> > (you
> > > also suggested the same thing yesterday evening).
> > 
> > PTAL.
> > 
> > I am not 100% about the indentation of the assert error message but it has
> been
> > done that way by the 'gn format' command.
> 
> We have to update our recipes I guess. :)

Actually no, this is generated from the MB. So you'll just have to clean up the
'shared' and 'static' mixins here:
https://chromium.googlesource.com/external/webrtc/+/master/tools-webrtc/mb/mb...

[mbonadei, 2017-03-02 18:33:03]

On 2017/03/02 02:32:15, kjellander_webrtc wrote:
> On 2017/03/01 18:54:09, mbonadei wrote:
> > On 2017/03/01 18:31:54, mbonadei wrote:
> > > On 2017/03/01 18:20:40, mbonadei wrote:
> > > > On 2017/03/01 17:39:27, mbonadei wrote:
> > > > > Using default_args = {} works. :)
> > > > 
> > > > Let me add an assertion as suggested by dprake@ on the gn-dev mailing
list
> > > (you
> > > > also suggested the same thing yesterday evening).
> > > 
> > > PTAL.
> > > 
> > > I am not 100% about the indentation of the assert error message but it has
> > been
> > > done that way by the 'gn format' command.
> > 
> > We have to update our recipes I guess. :)
> 
> Actually no, this is generated from the MB. So you'll just have to clean up
the
> 'shared' and 'static' mixins here:
>
https://chromium.googlesource.com/external/webrtc/+/master/tools-webrtc/mb/mb...

PTAL.

[kjellander_webrtc, 2017-03-03 04:19:02]

https://codereview.webrtc.org/2728643003/diff/60001/webrtc/webrtc.gni
File webrtc/webrtc.gni (right):

https://codereview.webrtc.org/2728643003/diff/60001/webrtc/webrtc.gni#newcode18
webrtc/webrtc.gni:18: print("The Gn argument `is_component_build` is currently "
+
How about replacing line 17-29 with just:
assert(!is_component_build, 
   """Please avoid using `is_component_build=true`

The GN argument `is_component_build` is currently ignored for WebRTC builds.
Component builds are supported by Chromium and the argument
`is_component_build` makes it possible to create shared libraries instead of
static libraries. If an app depends on WebRTC it makes sense to just depend on
the WebRTC static library, so there is no difference between
`is_component_build=true` and `is_component_build=false`

More info about component builds at: 
https://chromium.googlesource.com/chromium/src/+/master/docs/component_build.md
""")

[mbonadei, 2017-03-03 18:03:59]

https://codereview.webrtc.org/2728643003/diff/60001/webrtc/webrtc.gni
File webrtc/webrtc.gni (right):

https://codereview.webrtc.org/2728643003/diff/60001/webrtc/webrtc.gni#newcode18
webrtc/webrtc.gni:18: print("The Gn argument `is_component_build` is currently "
+
On 2017/03/03 04:19:02, kjellander_webrtc wrote:
> How about replacing line 17-29 with just:
> assert(!is_component_build, 
>    """Please avoid using `is_component_build=true`
> 
> The GN argument `is_component_build` is currently ignored for WebRTC builds.
> Component builds are supported by Chromium and the argument
> `is_component_build` makes it possible to create shared libraries instead of
> static libraries. If an app depends on WebRTC it makes sense to just depend on
> the WebRTC static library, so there is no difference between
> `is_component_build=true` and `is_component_build=false`
> 
> More info about component builds at: 
>
https://chromium.googlesource.com/chromium/src/+/master/docs/component_build.md
> """)

Yeah, this would be cleaner than the current solution but it seems that newlines
are not supported in a string constant:

> gn gen out/default --args="is_component_build=true"
ERROR at //webrtc/webrtc.gni:18:10: Newline in string constant.
       """Please avoid using `is_component_build=true`.
         ^---------------------------------------------

And adding '\n' does not create a newline.

[kjellander_webrtc, 2017-03-03 19:10:03]

lgtm with the assert message updated.

https://codereview.webrtc.org/2728643003/diff/60001/webrtc/webrtc.gni
File webrtc/webrtc.gni (right):

https://codereview.webrtc.org/2728643003/diff/60001/webrtc/webrtc.gni#newcode18
webrtc/webrtc.gni:18: print("The Gn argument `is_component_build` is currently "
+
On 2017/03/03 18:03:59, mbonadei wrote:
> On 2017/03/03 04:19:02, kjellander_webrtc wrote:
> > How about replacing line 17-29 with just:
> > assert(!is_component_build, 
> >    """Please avoid using `is_component_build=true`
> > 
> > The GN argument `is_component_build` is currently ignored for WebRTC builds.
> > Component builds are supported by Chromium and the argument
> > `is_component_build` makes it possible to create shared libraries instead of
> > static libraries. If an app depends on WebRTC it makes sense to just depend
on
> > the WebRTC static library, so there is no difference between
> > `is_component_build=true` and `is_component_build=false`
> > 
> > More info about component builds at: 
> >
>
https://chromium.googlesource.com/chromium/src/+/master/docs/component_build.md
> > """)
> 
> Yeah, this would be cleaner than the current solution but it seems that
newlines
> are not supported in a string constant:
> 
> > gn gen out/default --args="is_component_build=true"
> ERROR at //webrtc/webrtc.gni:18:10: Newline in string constant.
>        """Please avoid using `is_component_build=true`.
>          ^---------------------------------------------
> 
> And adding '\n' does not create a newline.

I see, then this is fine.

https://codereview.webrtc.org/2728643003/diff/60001/webrtc/webrtc.gni#newcode28
webrtc/webrtc.gni:28: assert(!is_component_build, "Please avoid using
`is_component_build=true`.")
I suggest changing to "Please avoid using `Component builds are not supported in
WebRTC."

[mbonadei, 2017-03-03 19:34:20]

The CQ bit was checked by mbonadei@webrtc.org

[mbonadei, 2017-03-03 19:34:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2728643003/#ps80001 (title:
"Updating assertion error message.")

[commit-bot: I haz the power, 2017-03-03 19:34:22]

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

[commit-bot: I haz the power, 2017-03-03 19:36:38]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488569660189180,
"parent_rev": "02455b27a278370368ea44154dba5c1504d4f1d5", "commit_rev":
"2cb3944ba7e4467601ebd94888e1811a6d77be49"}

[commit-bot: I haz the power, 2017-03-03 19:36:40]

Description was changed from

==========
Setting is_component_build to false by default

Webrtc does not support component builds so we want to override
the chromium default value (which can be true on debug builds if the
os is different from iOS).

Please note that the user can set this value to true in two ways:

 - using --args (e.g.: gn gen out/default --args='is_component_build=true'
 - changing the value in the args.gn file

But in both cases the value will be ignored because we don't use the
'component' template but we rely directly on 'rtc_static_library' and
'rtc_shared_library'.

BUG=webrtc:6975
NOTRY=True
==========

to

==========
Setting is_component_build to false by default

Webrtc does not support component builds so we want to override
the chromium default value (which can be true on debug builds if the
os is different from iOS).

Please note that the user can set this value to true in two ways:

 - using --args (e.g.: gn gen out/default --args='is_component_build=true'
 - changing the value in the args.gn file

But in both cases the value will be ignored because we don't use the
'component' template but we rely directly on 'rtc_static_library' and
'rtc_shared_library'.

BUG=webrtc:6975
NOTRY=True

Review-Url: https://codereview.webrtc.org/2728643003
Cr-Commit-Position: refs/heads/master@{#17020}
Committed:
https://chromium.googlesource.com/external/webrtc/+/2cb3944ba7e4467601ebd9488...
==========

[commit-bot: I haz the power, 2017-03-03 19:36:41]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/2cb3944ba7e4467601ebd9488...

[tommi, 2017-03-04 00:14:05]

tommi@webrtc.org changed reviewers:
+ tommi@webrtc.org

[tommi, 2017-03-04 00:14:05]

https://codereview.webrtc.org/2728643003/diff/80001/webrtc/webrtc.gni
File webrtc/webrtc.gni (right):

https://codereview.webrtc.org/2728643003/diff/80001/webrtc/webrtc.gni#newcode28
webrtc/webrtc.gni:28: assert(!is_component_build, "Component builds are not
supported in WebRTC.")
Although webrtc ignores the switch, we can't break chrome when it's used. This
fails when rolling.

[tommi, 2017-03-04 00:14:47]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.webrtc.org/2731703004/ by tommi@webrtc.org.

The reason for reverting is: Breaks chrome rolls.

[mbonadei, 2017-03-04 03:36:19]

On 2017/03/04 00:14:05, tommi (webrtc) wrote:
> https://codereview.webrtc.org/2728643003/diff/80001/webrtc/webrtc.gni
> File webrtc/webrtc.gni (right):
> 
>
https://codereview.webrtc.org/2728643003/diff/80001/webrtc/webrtc.gni#newcode28
> webrtc/webrtc.gni:28: assert(!is_component_build, "Component builds are not
> supported in WebRTC.")
> Although webrtc ignores the switch, we can't break chrome when it's used. This
> fails when rolling.

Yes, my bad. I always forget to add 'build_with_chromium' to isolate them from
this kind of changes.

Thanks for reverting. I've just created
https://codereview.webrtc.org/2729243003/ to address the problem.