Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2845063002:
Hiding problematic targets from chromium (Closed)
|
Created:
3 years, 7 months ago by mbonadei Modified:
3 years, 7 months ago Reviewers:
kjellander_webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionHiding problematic targets from chromium
Targets that depend on //third_party/gflags are problematic in
chromium because it does not use gflags and if a target depends on
any target in //webrtc/test/BUILD.gn then GN will try to parse the
BUILD.gn file. Gn is eager in this phase of the build files generation
and if a target is not buildable it will complain.
In chromium obviously all the targets that depend on
//third_party/gflags are not buildable.
To be able to enable 'gn check' on the webrtc/base and
webrtc/test directories we have to hide them from chromium.
BUG=webrtc:6828
NOTRY=True
Review-Url: https://codereview.webrtc.org/2845063002
Cr-Commit-Position: refs/heads/master@{#17913}
Committed: https://chromium.googlesource.com/external/webrtc/+/9452c62f28d7159bfe87c5a06998ce86373610c1
Patch Set 1 #
Total comments: 2
Patch Set 2 : Moving everything under one condition #Patch Set 3 : moving test_support_unittests_resources to fix error #
Total comments: 2
Patch Set 4 : moving test_support_unittests_bundle_data in a better position #Messages
Total messages: 18 (10 generated)
Description was changed from ========== Enabling 'gn check' on webrtc/test directory This CL enables 'gn check' on the webrtc/test directory and it also hides problematic targets from chromium. Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build file generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 ========== to ========== Hiding problematic targets from chromium This CL enables 'gn check' on the webrtc/test directory and it also hides problematic targets from chromium. Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build file generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 ==========
Description was changed from ========== Hiding problematic targets from chromium This CL enables 'gn check' on the webrtc/test directory and it also hides problematic targets from chromium. Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build file generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 ========== to ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build file generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 ==========
Description was changed from ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build file generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 ========== to ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build file generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 NOTRY=True ==========
Description was changed from ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build file generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 NOTRY=True ========== to ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build files generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 NOTRY=True ==========
Description was changed from ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build files generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base directory we have to hide them from chromium. BUG=webrtc:6828 NOTRY=True ========== to ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build files generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base and webrtc/test directories we have to hide them from chromium. BUG=webrtc:6828 NOTRY=True ==========
mbonadei@webrtc.org changed reviewers: + kjellander@webrtc.org
https://codereview.webrtc.org/2845063002/diff/1/webrtc/test/BUILD.gn File webrtc/test/BUILD.gn (right): https://codereview.webrtc.org/2845063002/diff/1/webrtc/test/BUILD.gn#newcode136 webrtc/test/BUILD.gn:136: if (!build_with_chromium) { Let's move the lower targets up so we can avoid having multiple conditions and large comments.
https://codereview.webrtc.org/2845063002/diff/1/webrtc/test/BUILD.gn File webrtc/test/BUILD.gn (right): https://codereview.webrtc.org/2845063002/diff/1/webrtc/test/BUILD.gn#newcode136 webrtc/test/BUILD.gn:136: if (!build_with_chromium) { On 2017/04/27 12:39:44, kjellander_webrtc wrote: > Let's move the lower targets up so we can avoid having multiple conditions and > large comments. Done.
On 2017/04/27 12:46:43, mbonadei wrote: > https://codereview.webrtc.org/2845063002/diff/1/webrtc/test/BUILD.gn > File webrtc/test/BUILD.gn (right): > > https://codereview.webrtc.org/2845063002/diff/1/webrtc/test/BUILD.gn#newcode136 > webrtc/test/BUILD.gn:136: if (!build_with_chromium) { > On 2017/04/27 12:39:44, kjellander_webrtc wrote: > > Let's move the lower targets up so we can avoid having multiple conditions and > > large comments. > > Done. PS#2 has an error. I will upload PS#3 soon.
lgtm with a suggestion https://codereview.webrtc.org/2845063002/diff/40001/webrtc/test/BUILD.gn File webrtc/test/BUILD.gn (right): https://codereview.webrtc.org/2845063002/diff/40001/webrtc/test/BUILD.gn#newc... webrtc/test/BUILD.gn:307: if (is_ios) { Isn't it nicer to also move this up to under test_support_unittests_resources?
https://codereview.webrtc.org/2845063002/diff/40001/webrtc/test/BUILD.gn File webrtc/test/BUILD.gn (right): https://codereview.webrtc.org/2845063002/diff/40001/webrtc/test/BUILD.gn#newc... webrtc/test/BUILD.gn:307: if (is_ios) { On 2017/04/27 16:24:54, kjellander_webrtc wrote: > Isn't it nicer to also move this up to under test_support_unittests_resources? Yep, good advice. Done.
The CQ bit was checked by mbonadei@webrtc.org
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/2845063002/#ps60001 (title: "moving test_support_unittests_bundle_data in a better position")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493321192690200,
"parent_rev": "ec3f538bf5113898dd0a9a26aa87da8b5ce60fc4", "commit_rev":
"9452c62f28d7159bfe87c5a06998ce86373610c1"}
Message was sent while issue was closed.
Description was changed from ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build files generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base and webrtc/test directories we have to hide them from chromium. BUG=webrtc:6828 NOTRY=True ========== to ========== Hiding problematic targets from chromium Targets that depend on //third_party/gflags are problematic in chromium because it does not use gflags and if a target depends on any target in //webrtc/test/BUILD.gn then GN will try to parse the BUILD.gn file. Gn is eager in this phase of the build files generation and if a target is not buildable it will complain. In chromium obviously all the targets that depend on //third_party/gflags are not buildable. To be able to enable 'gn check' on the webrtc/base and webrtc/test directories we have to hide them from chromium. BUG=webrtc:6828 NOTRY=True Review-Url: https://codereview.webrtc.org/2845063002 Cr-Commit-Position: refs/heads/master@{#17913} Committed: https://chromium.googlesource.com/external/webrtc/+/9452c62f28d7159bfe87c5a06... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/external/webrtc/+/9452c62f28d7159bfe87c5a06... |
