New issue
Advanced search Search tips

Issue 824831 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

//third_party/webrtc/* does not pass GN header checks

Project Member Reported by w...@chromium.org, Mar 22 2018

Issue description

There are a few failures, several of which look like mismatches between audio_codec_defines and audio_codec_deps.
 
Ok, we do have gn --check enabled for all of WebRTC (with very few exceptions). How did you produce the errors you describe?

Comment 2 by w...@chromium.org, Apr 16 2018

Re #1: This was adding //third_party/webrtc/* to Chromium's list of things to GN check.
Status: Available (was: Untriaged)
Ok, we should fix that some time.
Cc: mbonadei@chromium.org
Status: Assigned (was: Available)
Cc: -mbonadei@chromium.org
Owner: mbonadei@chromium.org
Fixed by (not sure why the bug was not automatically updated):

https://webrtc-review.googlesource.com/97620
https://webrtc-review.googlesource.com/97622

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 4

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/01cd853d2ef17474a87c65b187213b4e55321d9b

commit 01cd853d2ef17474a87c65b187213b4e55321d9b
Author: Mirko Bonadei <mbonadei@webrtc.org>
Date: Tue Sep 04 09:45:56 2018

Add nogncheck for headers of codecs not used in Chromium.

iLBC [1] and Red [2] are not used in Chromium, this means that WebRTC
doeesn't add the GN dependency on them but the include checker
complains because when it parses the code it sees iLBC and Red headers
included (the GN checker doesn't run the c preprocessor).

[1] - https://cs.chromium.org/chromium/src/.gn?l=62-65&rcl=3f6c31f0fdba128d810c4e3e391fae3b1aca7e7c
[2] - https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_coding/audio_coding.gni?l=28-30&rcl=eb254b40b33380fcec43028dd89f3f6bab3d08a7

Bug:  chromium:824831 
Change-Id: I5059c1773fbe35f568c2fe3d8db3807f29973e7e
Reviewed-on: https://webrtc-review.googlesource.com/97620
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24546}
[modify] https://crrev.com/01cd853d2ef17474a87c65b187213b4e55321d9b/modules/audio_coding/acm2/rent_a_codec.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82fb538911c5a61d7a463370aeab68a36a74722c

commit 82fb538911c5a61d7a463370aeab68a36a74722c
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Sep 04 13:41:20 2018

Roll src/third_party/webrtc 4e5af96606b9..4092cd6db459 (8 commits)

https://webrtc.googlesource.com/src.git/+log/4e5af96606b9..4092cd6db459


git log 4e5af96606b9..4092cd6db459 --date=short --no-merges --format='%ad %ae %s'
2018-09-04 jonasolsson@webrtc.org Consolidate loggability checks and replace streams.
2018-09-04 mbonadei@webrtc.org Add nogncheck for headers of codecs not used in Chromium.
2018-09-04 ilnik@webrtc.org Reland "Remove obsolete field trial from the tests"
2018-09-04 ilnik@webrtc.org Revert "Remove obsolete field trial from the tests"
2018-09-04 sakal@webrtc.org Enable ULPFEC for kVideoCodecGeneric if GenericPictureId is enabled.
2018-09-04 magjed@webrtc.org Reland "Fix a bug in barcode_decoder.py"
2018-09-04 nisse@webrtc.org Delete class EventTimerWrapper.
2018-09-03 brandtr@webrtc.org Remove MediaOptimization::Reset.


Created with:
  gclient setdep -r src/third_party/webrtc@4092cd6db459

The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG= chromium:824831 
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Idabeba5988358f506170e35d914296813b5ba935
Reviewed-on: https://chromium-review.googlesource.com/1204052
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#588502}
[modify] https://crrev.com/82fb538911c5a61d7a463370aeab68a36a74722c/DEPS

Can we enable GN check for third_party/webrtc/* now?
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 4

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/01cd853d2ef17474a87c65b187213b4e55321d9b

commit 01cd853d2ef17474a87c65b187213b4e55321d9b
Author: Mirko Bonadei <mbonadei@webrtc.org>
Date: Tue Sep 04 09:45:56 2018

Add nogncheck for headers of codecs not used in Chromium.

iLBC [1] and Red [2] are not used in Chromium, this means that WebRTC
doeesn't add the GN dependency on them but the include checker
complains because when it parses the code it sees iLBC and Red headers
included (the GN checker doesn't run the c preprocessor).

[1] - https://cs.chromium.org/chromium/src/.gn?l=62-65&rcl=3f6c31f0fdba128d810c4e3e391fae3b1aca7e7c
[2] - https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_coding/audio_coding.gni?l=28-30&rcl=eb254b40b33380fcec43028dd89f3f6bab3d08a7

Bug:  chromium:824831 
Change-Id: I5059c1773fbe35f568c2fe3d8db3807f29973e7e
Reviewed-on: https://webrtc-review.googlesource.com/97620
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24546}
[modify] https://crrev.com/01cd853d2ef17474a87c65b187213b4e55321d9b/modules/audio_coding/acm2/rent_a_codec.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82fb538911c5a61d7a463370aeab68a36a74722c

commit 82fb538911c5a61d7a463370aeab68a36a74722c
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Sep 04 13:41:20 2018

Roll src/third_party/webrtc 4e5af96606b9..4092cd6db459 (8 commits)

https://webrtc.googlesource.com/src.git/+log/4e5af96606b9..4092cd6db459


git log 4e5af96606b9..4092cd6db459 --date=short --no-merges --format='%ad %ae %s'
2018-09-04 jonasolsson@webrtc.org Consolidate loggability checks and replace streams.
2018-09-04 mbonadei@webrtc.org Add nogncheck for headers of codecs not used in Chromium.
2018-09-04 ilnik@webrtc.org Reland "Remove obsolete field trial from the tests"
2018-09-04 ilnik@webrtc.org Revert "Remove obsolete field trial from the tests"
2018-09-04 sakal@webrtc.org Enable ULPFEC for kVideoCodecGeneric if GenericPictureId is enabled.
2018-09-04 magjed@webrtc.org Reland "Fix a bug in barcode_decoder.py"
2018-09-04 nisse@webrtc.org Delete class EventTimerWrapper.
2018-09-03 brandtr@webrtc.org Remove MediaOptimization::Reset.


Created with:
  gclient setdep -r src/third_party/webrtc@4092cd6db459

The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG= chromium:824831 
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Idabeba5988358f506170e35d914296813b5ba935
Reviewed-on: https://chromium-review.googlesource.com/1204052
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#588502}
[modify] https://crrev.com/82fb538911c5a61d7a463370aeab68a36a74722c/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 4

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/01cd853d2ef17474a87c65b187213b4e55321d9b

commit 01cd853d2ef17474a87c65b187213b4e55321d9b
Author: Mirko Bonadei <mbonadei@webrtc.org>
Date: Tue Sep 04 09:45:56 2018

Add nogncheck for headers of codecs not used in Chromium.

iLBC [1] and Red [2] are not used in Chromium, this means that WebRTC
doeesn't add the GN dependency on them but the include checker
complains because when it parses the code it sees iLBC and Red headers
included (the GN checker doesn't run the c preprocessor).

[1] - https://cs.chromium.org/chromium/src/.gn?l=62-65&rcl=3f6c31f0fdba128d810c4e3e391fae3b1aca7e7c
[2] - https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_coding/audio_coding.gni?l=28-30&rcl=eb254b40b33380fcec43028dd89f3f6bab3d08a7

Bug:  chromium:824831 
Change-Id: I5059c1773fbe35f568c2fe3d8db3807f29973e7e
Reviewed-on: https://webrtc-review.googlesource.com/97620
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24546}
[modify] https://crrev.com/01cd853d2ef17474a87c65b187213b4e55321d9b/modules/audio_coding/acm2/rent_a_codec.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82fb538911c5a61d7a463370aeab68a36a74722c

commit 82fb538911c5a61d7a463370aeab68a36a74722c
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Sep 04 13:41:20 2018

Roll src/third_party/webrtc 4e5af96606b9..4092cd6db459 (8 commits)

https://webrtc.googlesource.com/src.git/+log/4e5af96606b9..4092cd6db459


git log 4e5af96606b9..4092cd6db459 --date=short --no-merges --format='%ad %ae %s'
2018-09-04 jonasolsson@webrtc.org Consolidate loggability checks and replace streams.
2018-09-04 mbonadei@webrtc.org Add nogncheck for headers of codecs not used in Chromium.
2018-09-04 ilnik@webrtc.org Reland "Remove obsolete field trial from the tests"
2018-09-04 ilnik@webrtc.org Revert "Remove obsolete field trial from the tests"
2018-09-04 sakal@webrtc.org Enable ULPFEC for kVideoCodecGeneric if GenericPictureId is enabled.
2018-09-04 magjed@webrtc.org Reland "Fix a bug in barcode_decoder.py"
2018-09-04 nisse@webrtc.org Delete class EventTimerWrapper.
2018-09-03 brandtr@webrtc.org Remove MediaOptimization::Reset.


Created with:
  gclient setdep -r src/third_party/webrtc@4092cd6db459

The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG= chromium:824831 
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Idabeba5988358f506170e35d914296813b5ba935
Reviewed-on: https://chromium-review.googlesource.com/1204052
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#588502}
[modify] https://crrev.com/82fb538911c5a61d7a463370aeab68a36a74722c/DEPS

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/822928a04b2d206dfd49b2d467bc734f817f0f66

commit 822928a04b2d206dfd49b2d467bc734f817f0f66
Author: Mirko Bonadei <mbonadei@webrtc.org>
Date: Wed Sep 05 08:24:27 2018

Fix rtc_base_generic webrtc_overrides dependencies.

This change is needed in order to land
https://chromium-review.googlesource.com/c/chromium/src/+/1203773.

Bug:  chromium:824831 
Change-Id: Ie37a099fa6059039f53feb76f1c640610ca25696
Reviewed-on: https://webrtc-review.googlesource.com/97940
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24565}
[modify] https://crrev.com/822928a04b2d206dfd49b2d467bc734f817f0f66/rtc_base/BUILD.gn

Project Member

Comment 15 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e0b2bda34484ebadc8c564ea70da4b0bf282c951

commit e0b2bda34484ebadc8c564ea70da4b0bf282c951
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Sep 04 17:13:18 2018

Roll src/third_party/webrtc 4092cd6db459..46d65e2c2cc0 (6 commits)

https://webrtc.googlesource.com/src.git/+log/4092cd6db459..46d65e2c2cc0


git log 4092cd6db459..46d65e2c2cc0 --date=short --no-merges --format='%ad %ae %s'
2018-09-04 jonasolsson@webrtc.org Revert "Consolidate loggability checks and replace streams."
2018-09-04 nisse@webrtc.org Revert "Reland "Refactor TestAudioDeviceModule to not depend on EventTimerWrapper.""
2018-09-04 nisse@webrtc.org Revert "Delete class EventTimerWrapper."
2018-09-04 henrika@webrtc.org Adds WebRTC.Audio.Record/PlayoutSampleRateOffsetInPercent UMA stats to native WebRTC.
2018-09-04 mbonadei@webrtc.org Upload WebRTC CLs from Chromium.
2018-09-04 mbonadei@webrtc.org Remove check_includes=false from audio_device_impl.


Created with:
  gclient setdep -r src/third_party/webrtc@46d65e2c2cc0

The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG=chromium:b/113648245,chromium:824831
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I7ab022d3ba50c8a1fb462574e1922f6a3f2b17b7
Reviewed-on: https://chromium-review.googlesource.com/1204060
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#588559}
[modify] https://crrev.com/e0b2bda34484ebadc8c564ea70da4b0bf282c951/DEPS

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa3506e256d2ea30ebc28a03f152f26ceed55335

commit aa3506e256d2ea30ebc28a03f152f26ceed55335
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Sep 05 13:14:02 2018

Roll src/third_party/webrtc 711a31aead90..b29409064e87 (12 commits)

https://webrtc.googlesource.com/src.git/+log/711a31aead90..b29409064e87


git log 711a31aead90..b29409064e87 --date=short --no-merges --format='%ad %ae %s'
2018-09-05 yinwa@webrtc.org Revert "Explicitly wrap main thread in test_main.cc."
2018-09-05 magjed@webrtc.org Reland "Add Y4mFileReader"
2018-09-05 srte@webrtc.org Minor refactor in GoogCcNetworkController.
2018-09-05 phoglund@webrtc.org Revert "Adds WebRTC.Audio.Record/PlayoutSampleRateOffsetInPercent UMA stats to native WebRTC."
2018-09-05 andersc@webrtc.org Scaling settings nullability in Obj-C SDK.
2018-09-05 mbonadei@webrtc.org Remove clang:find_bad_constructs suppression from logging (part 1).
2018-09-05 yinwa@webrtc.org Revert "Remove steveanton from api/ watchlist"
2018-09-05 mbonadei@webrtc.org Fix rtc_base_generic webrtc_overrides dependencies.
2018-09-05 jonasolsson@webrtc.org Remove stringstreams from sdp parsing.
2018-09-05 yinwa@webrtc.org Revert "Add SSLConfig object to IceServer."
2018-09-05 srte@webrtc.org Adds arithmetic support to DataRate.
2018-09-05 sprang@webrtc.org Don't let time flow backwards in pacer


Created with:
  gclient setdep -r src/third_party/webrtc@b29409064e87

The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG=chromium:b/113648245,chromium:None,chromium:824831
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ifc8096f1b03f6ba7c906d891f695548609660f4a
Reviewed-on: https://chromium-review.googlesource.com/1205628
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#588845}
[modify] https://crrev.com/aa3506e256d2ea30ebc28a03f152f26ceed55335/DEPS

Project Member

Comment 17 by bugdroid1@chromium.org, Sep 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae3ab57c1e4342162d4bd81cdb49ae7e36d441eb

commit ae3ab57c1e4342162d4bd81cdb49ae7e36d441eb
Author: Mirko Bonadei <mbonadei@chromium.org>
Date: Mon Sep 10 19:21:16 2018

Enable "gn check" on //third_party/webrtc.

Problems have been fixed by:
- https://webrtc-review.googlesource.com/97620
- https://webrtc-review.googlesource.com/97622

Bug:  824831 
Change-Id: Ibe2f12f6d935c61a37cd253d22960a1ccba9f50a
Reviewed-on: https://chromium-review.googlesource.com/1203773
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Mirko Bonadei <mbonadei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590016}
[modify] https://crrev.com/ae3ab57c1e4342162d4bd81cdb49ae7e36d441eb/.gn

Status: Fixed (was: Assigned)

Sign in to add a comment