Make video_quality_measurement buildable again.

Make video_quality_measurement buildable again.

BUG=None

Review-Url: https://codereview.webrtc.org/2651543008
Cr-Commit-Position: refs/heads/master@{#16290}
Committed: https://chromium.googlesource.com/external/webrtc/+/52cdd3bb305190d287db73ee29bc4370a5ce2788

[brandtr, 2017-01-25 14:13:42]

brandtr@webrtc.org changed reviewers:
+ kjellander@webrtc.org, sprang@webrtc.org

[brandtr, 2017-01-25 14:13:42]

PTAL.

kjellander: BUILD.gn.
sprang: OWNER.

[kjellander_webrtc, 2017-01-25 15:25:21]

lgtm

[sprang_webrtc, 2017-01-25 15:27:42]

lgtm

[brandtr, 2017-01-25 15:31:55]

The CQ bit was checked by brandtr@webrtc.org

[commit-bot: I haz the power, 2017-01-25 15:32:08]

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-01-25 15:36:22]

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

[commit-bot: I haz the power, 2017-01-25 15:36:23]

Try jobs failed on following builders:
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/18117)

[brandtr, 2017-01-25 16:01:12]

Apparently I didn't run a final 'gn check' before submitting. Doing so gives
this error:
========
 brandtr@brandtr  ~/src/webrtc/src/out   WIP-Build_video_quality_measurement
 gn check Debug
ERROR at
//webrtc/modules/video_coding/codecs/tools/video_quality_measurement.cc:30:11:
Can't include this header from here.
#include "webrtc/system_wrappers/include/trace.h"
          ^-------------------------------------
The target:
  //webrtc/modules/video_coding:video_quality_measurement
is including a file from the target:
  //webrtc/system_wrappers:system_wrappers

It's usually best to depend directly on the destination target.
In some cases, the destination target is considered a subcomponent
of an intermediate target. In this case, the intermediate target
should depend publicly on the destination to forward the ability
to include headers.

Dependency chain (there may also be others):
  //webrtc/modules/video_coding:video_quality_measurement -->
  //webrtc/modules/video_coding:video_codecs_test_framework --[private]-->
  //webrtc/system_wrappers:system_wrappers
========

I thought depending on 'system_wrappers:system_wrappers_default' would include
'system_wrappers:system_wrappers', but apparently not? Not sure if the new way I
did it is correct, would you mind having a second look kjellander@?

[kjellander_webrtc, 2017-01-25 16:13:52]

On 2017/01/25 16:01:12, brandtr wrote:
> Apparently I didn't run a final 'gn check' before submitting. Doing so gives
> this error:
> ========
>  brandtr@brandtr  ~/src/webrtc/src/out  
WIP-Build_video_quality_measurement
>  gn check Debug
> ERROR at
> //webrtc/modules/video_coding/codecs/tools/video_quality_measurement.cc:30:11:
> Can't include this header from here.
> #include "webrtc/system_wrappers/include/trace.h"
>           ^-------------------------------------
> The target:
>   //webrtc/modules/video_coding:video_quality_measurement
> is including a file from the target:
>   //webrtc/system_wrappers:system_wrappers
> 
> It's usually best to depend directly on the destination target.
> In some cases, the destination target is considered a subcomponent
> of an intermediate target. In this case, the intermediate target
> should depend publicly on the destination to forward the ability
> to include headers.
> 
> Dependency chain (there may also be others):
>   //webrtc/modules/video_coding:video_quality_measurement -->
>   //webrtc/modules/video_coding:video_codecs_test_framework --[private]-->
>   //webrtc/system_wrappers:system_wrappers
> ========
> 
> I thought depending on 'system_wrappers:system_wrappers_default' would include
> 'system_wrappers:system_wrappers', but apparently not? Not sure if the new way
I
> did it is correct, would you mind having a second look kjellander@?

I believe it does, but GN check demands that you do explicit declarations to
avoid deep chains of dependencies that are hard to overview. I believe that's
why it complained.
still lgtm

[brandtr, 2017-01-25 16:17:59]

On 2017/01/25 16:13:52, kjellander_webrtc wrote:
> On 2017/01/25 16:01:12, brandtr wrote:
> > Apparently I didn't run a final 'gn check' before submitting. Doing so gives
> > this error:
> > ========
> >  brandtr@brandtr  ~/src/webrtc/src/out  
> WIP-Build_video_quality_measurement
> >  gn check Debug
> > ERROR at
> >
//webrtc/modules/video_coding/codecs/tools/video_quality_measurement.cc:30:11:
> > Can't include this header from here.
> > #include "webrtc/system_wrappers/include/trace.h"
> >           ^-------------------------------------
> > The target:
> >   //webrtc/modules/video_coding:video_quality_measurement
> > is including a file from the target:
> >   //webrtc/system_wrappers:system_wrappers
> > 
> > It's usually best to depend directly on the destination target.
> > In some cases, the destination target is considered a subcomponent
> > of an intermediate target. In this case, the intermediate target
> > should depend publicly on the destination to forward the ability
> > to include headers.
> > 
> > Dependency chain (there may also be others):
> >   //webrtc/modules/video_coding:video_quality_measurement -->
> >   //webrtc/modules/video_coding:video_codecs_test_framework --[private]-->
> >   //webrtc/system_wrappers:system_wrappers
> > ========
> > 
> > I thought depending on 'system_wrappers:system_wrappers_default' would
include
> > 'system_wrappers:system_wrappers', but apparently not? Not sure if the new
way
> I
> > did it is correct, would you mind having a second look kjellander@?
> 
> I believe it does, but GN check demands that you do explicit declarations to
> avoid deep chains of dependencies that are hard to overview. I believe that's
> why it complained.
> still lgtm

Ok, thanks for the explanation.

[brandtr, 2017-01-25 16:18:03]

The CQ bit was checked by brandtr@webrtc.org

[brandtr, 2017-01-25 16:18:03]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2651543008/#ps20001 (title:
"Make dependence on system_wrappers:system_wrappers explicit.")

[commit-bot: I haz the power, 2017-01-25 16:18:12]

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-01-25 16:24:02]

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

[commit-bot: I haz the power, 2017-01-25 16:24:03]

Try jobs failed on following builders:
  ios64_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios9_dbg/buil...)

[kjellander_webrtc, 2017-01-25 16:44:03]

On 2017/01/25 16:24:03, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   ios64_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios9_dbg/buil...)

Seems you need webrtc/system_wrappers:field_trial_default as well.

[brandtr_google, 2017-01-26 09:22:07]

The CQ bit was checked by brandtr@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 09:22:11]

Dry run: 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-01-26 10:19:46]

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

[commit-bot: I haz the power, 2017-01-26 10:19:47]

Dry run: This issue passed the CQ dry run.

[brandtr, 2017-01-26 10:22:39]

On 2017/01/25 16:44:03, kjellander_webrtc wrote:
> On 2017/01/25 16:24:03, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   ios64_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios9_dbg/buil...)
> 
> Seems you need webrtc/system_wrappers:field_trial_default as well.

Yep, thanks :)

[brandtr, 2017-01-26 10:22:43]

The CQ bit was checked by brandtr@webrtc.org

[brandtr, 2017-01-26 10:22:44]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org,
kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2651543008/#ps40001 (title:
"Make iOS happy.")

[commit-bot: I haz the power, 2017-01-26 10:22:46]

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-01-26 10:25:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485426163798020,
"parent_rev": "f3d622d4ade9364259d8028aee39c68f41c86918", "commit_rev":
"52cdd3bb305190d287db73ee29bc4370a5ce2788"}

[commit-bot: I haz the power, 2017-01-26 10:25:23]

Description was changed from

==========
Make video_quality_measurement buildable again.

BUG=None
==========

to

==========
Make video_quality_measurement buildable again.

BUG=None

Review-Url: https://codereview.webrtc.org/2651543008
Cr-Commit-Position: refs/heads/master@{#16290}
Committed:
https://chromium.googlesource.com/external/webrtc/+/52cdd3bb305190d287db73ee2...
==========

[commit-bot: I haz the power, 2017-01-26 10:25:25]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/52cdd3bb305190d287db73ee2...