[Chromecast] Don't use static c++ libraries for clang builds

[Chromecast] Don't use static c++ libraries for clang builds

BUG=internal b/25696204
TEST=Build and run netflix on device

Review-Url: https://codereview.chromium.org/2726013002
Cr-Commit-Position: refs/heads/master@{#455193}
Committed: https://chromium.googlesource.com/chromium/src/+/e03d6f332a76e587d802634757c3e9e74778d5aa

[bcf, 2017-03-01 23:59:09]

Description was changed from

==========
[Chromecast] Don't use static c++ libraries for clang builds

BUG=internal b/25696204
TEST=Build and run netflix on device
==========

to

==========
[Chromecast] Don't use static c++ libraries for clang builds

BUG=internal b/25696204
TEST=Build and run netflix on device
==========

[bcf, 2017-03-01 23:59:09]

bcf@chromium.org changed reviewers:
+ mbjorge@chromium.org, slan@chromium.org, wzhong@chromium.org

[bcf, 2017-03-01 23:59:20]

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
File build/config/chromecast/BUILD.gn (right):

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
build/config/chromecast/BUILD.gn:14: "-Wl,--exclude-libs=libffmpeg.a",
This shouldn't be necessary since we're not using "-Wl,--export-dynamic"
anymore.

[wzhong, 2017-03-02 00:12:40]

lgtm

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
File build/config/chromecast/BUILD.gn (right):

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
build/config/chromecast/BUILD.gn:14: "-Wl,--exclude-libs=libffmpeg.a",
On 2017/03/01 23:59:20, bcf wrote:
> This shouldn't be necessary since we're not using "-Wl,--export-dynamic"
> anymore.

Not really. I believe in this case, without this, symbols will be visibility and
merged at runtime. We don't want them to merge.

[halliwell, 2017-03-02 00:28:58]

On 2017/03/02 00:12:40, wzhong wrote:
> lgtm
> 
>
https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
> File build/config/chromecast/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
> build/config/chromecast/BUILD.gn:14: "-Wl,--exclude-libs=libffmpeg.a",
> On 2017/03/01 23:59:20, bcf wrote:
> > This shouldn't be necessary since we're not using "-Wl,--export-dynamic"
> > anymore.
> 
> Not really. I believe in this case, without this, symbols will be visibility
and
> merged at runtime. We don't want them to merge.

lgtm

[mbjorge, 2017-03-02 02:35:10]

mbjorge@chromium.org changed reviewers:
+ halliwell@chromium.org

[mbjorge, 2017-03-02 02:35:11]

lgtm

[bcf, 2017-03-02 02:59:26]

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
File build/config/chromecast/BUILD.gn (right):

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
build/config/chromecast/BUILD.gn:14: "-Wl,--exclude-libs=libffmpeg.a",
On 2017/03/02 00:12:40, wzhong wrote:
> On 2017/03/01 23:59:20, bcf wrote:
> > This shouldn't be necessary since we're not using "-Wl,--export-dynamic"
> > anymore.
> 
> Not really. I believe in this case, without this, symbols will be visibility
and
> merged at runtime. We don't want them to merge.

So you're saying we still need this?

I thought the original purpose cause of this was '-Wl,--export-dynamic' being
added. This CL takes that away.

[wzhong, 2017-03-02 15:53:49]

lgtm

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
File build/config/chromecast/BUILD.gn (right):

https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
build/config/chromecast/BUILD.gn:14: "-Wl,--exclude-libs=libffmpeg.a",
On 2017/03/02 02:59:26, bcf wrote:
> On 2017/03/02 00:12:40, wzhong wrote:
> > On 2017/03/01 23:59:20, bcf wrote:
> > > This shouldn't be necessary since we're not using "-Wl,--export-dynamic"
> > > anymore.
> > 
> > Not really. I believe in this case, without this, symbols will be visibility
> and
> > merged at runtime. We don't want them to merge.
> 
> So you're saying we still need this?
> 
> I thought the original purpose cause of this was '-Wl,--export-dynamic' being
> added. This CL takes that away.

Probably you're right.

The caveat is the symbols causing trouble here are written in assembly so please
verify on Chirp to see if multiple instances of ffmpeg work.

[slan, 2017-03-02 16:31:47]

lgtm

[bcf, 2017-03-02 22:05:39]

On 2017/03/02 15:53:49, wzhong wrote:
> lgtm
> 
>
https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
> File build/config/chromecast/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2726013002/diff/1/build/config/chromecast/BUI...
> build/config/chromecast/BUILD.gn:14: "-Wl,--exclude-libs=libffmpeg.a",
> On 2017/03/02 02:59:26, bcf wrote:
> > On 2017/03/02 00:12:40, wzhong wrote:
> > > On 2017/03/01 23:59:20, bcf wrote:
> > > > This shouldn't be necessary since we're not using "-Wl,--export-dynamic"
> > > > anymore.
> > > 
> > > Not really. I believe in this case, without this, symbols will be
visibility
> > and
> > > merged at runtime. We don't want them to merge.
> > 
> > So you're saying we still need this?
> > 
> > I thought the original purpose cause of this was '-Wl,--export-dynamic'
being
> > added. This CL takes that away.
> 
> Probably you're right.
> 
> The caveat is the symbols causing trouble here are written in assembly so
please
> verify on Chirp to see if multiple instances of ffmpeg work.

Symbol list seems to look good.

[bcf, 2017-03-02 22:05:45]

The CQ bit was checked by bcf@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 22:06:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 22:59:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 22:59:54]

Dry run: This issue passed the CQ dry run.

[bcf, 2017-03-02 23:06:37]

The CQ bit was checked by bcf@chromium.org

[commit-bot: I haz the power, 2017-03-02 23:07:06]

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

[commit-bot: I haz the power, 2017-03-02 23:18:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 23:18:41]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[bcf, 2017-03-02 23:21:25]

bcf@chromium.org changed reviewers:
+ brettw@chromium.org, dpranke@google.com

[bcf, 2017-03-06 19:13:03]

On 2017/03/02 23:18:41, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Ping for OWNERS.

[Dirk Pranke, 2017-03-07 00:16:43]

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

[Dirk Pranke, 2017-03-07 00:16:43]

lgtm

[bcf, 2017-03-07 00:18:25]

The CQ bit was checked by bcf@chromium.org

[commit-bot: I haz the power, 2017-03-07 00:19:17]

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

[commit-bot: I haz the power, 2017-03-07 01:52:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 01:52:38]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[bcf, 2017-03-07 03:14:51]

The CQ bit was checked by bcf@chromium.org

[commit-bot: I haz the power, 2017-03-07 03:15:22]

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

[commit-bot: I haz the power, 2017-03-07 05:30:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 05:30:59]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[bcf, 2017-03-07 18:38:19]

The CQ bit was checked by bcf@chromium.org

[commit-bot: I haz the power, 2017-03-07 18:39:12]

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

[commit-bot: I haz the power, 2017-03-07 20:17:54]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1488911899176400, "parent_rev":
"1b26f93c8009b26acc32161860c6dd1aac3d9818", "commit_rev":
"e03d6f332a76e587d802634757c3e9e74778d5aa"}

[commit-bot: I haz the power, 2017-03-07 20:18:39]

Description was changed from

==========
[Chromecast] Don't use static c++ libraries for clang builds

BUG=internal b/25696204
TEST=Build and run netflix on device
==========

to

==========
[Chromecast] Don't use static c++ libraries for clang builds

BUG=internal b/25696204
TEST=Build and run netflix on device

Review-Url: https://codereview.chromium.org/2726013002
Cr-Commit-Position: refs/heads/master@{#455193}
Committed:
https://chromium.googlesource.com/chromium/src/+/e03d6f332a76e587d802634757c3...
==========

[commit-bot: I haz the power, 2017-03-07 20:18:40]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/e03d6f332a76e587d802634757c3...