Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2809303002:
Fix extensions targets building without chrome (Closed)
DescriptionFix extensions targets building without chrome
The extensions:extensions_renderer_resources_grit target fails to
build without building parts of chrome first, meaning that targets in
//extensions fail to build. This includes binaries like
extensions_unittests, but also anything that depends on
extensions/common and not chrome. The dependency was introduced in
https://codereview.chromium.org/2727123002 without knowing it would
cause build problems.
This change seems to ensure the additional necessary mojo files are
generated before being used, without introducing additional major
chrome dependencies on top of those already here.
Specifically, it ensures that
obj/extensions/extensions_renderer_resources.ninja includes the
mojo_bindings_common__* results.
BUG=706786
R=rdevlin.cronin@chromium.org
Review-Url: https://codereview.chromium.org/2809303002
Cr-Commit-Position: refs/heads/master@{#463870}
Committed: https://chromium.googlesource.com/chromium/src/+/01d3aa8fc776bb4c2ca45d8e77e1f120e7d61f1b
Patch Set 1 #
Total comments: 2
Patch Set 2 : generator #Messages
Total messages: 17 (10 generated)
Description was changed from ========== Fix extensions targets building without chrome The extensions:extensions_renderer_resources_grit target fails to build without building parts of chrome first, meaning that targets in //extensions fail to build. This includes binaries like extensions_unittests, but also anything that depends on extensions/common and not chrome. The dependency was introduced in https://codereview.chromium.org/2727123002 without knowing it would cause build problems. This change seems to ensure the additional necessary mojo files are generated before being used, without introducing additional major chrome dependencies on top of those already here. Specifically, it ensures that obj/extensions/extensions_renderer_resources.ninja includes the mojo_bindings_common__* results. BUG=706786 R=rdevlin.cronin@chromium.org ========== to ========== Fix extensions targets building without chrome The extensions:extensions_renderer_resources_grit target fails to build without building parts of chrome first, meaning that targets in //extensions fail to build. This includes binaries like extensions_unittests, but also anything that depends on extensions/common and not chrome. The dependency was introduced in https://codereview.chromium.org/2727123002 without knowing it would cause build problems. This change seems to ensure the additional necessary mojo files are generated before being used, without introducing additional major chrome dependencies on top of those already here. Specifically, it ensures that obj/extensions/extensions_renderer_resources.ninja includes the mojo_bindings_common__* results. BUG=706786 R=rdevlin.cronin@chromium.org ==========
michaelpg@chromium.org changed reviewers: + takumif@chromium.org
Description was changed from ========== Fix extensions targets building without chrome The extensions:extensions_renderer_resources_grit target fails to build without building parts of chrome first, meaning that targets in //extensions fail to build. This includes binaries like extensions_unittests, but also anything that depends on extensions/common and not chrome. The dependency was introduced in https://codereview.chromium.org/2727123002 without knowing it would cause build problems. This change seems to ensure the additional necessary mojo files are generated before being used, without introducing additional major chrome dependencies on top of those already here. Specifically, it ensures that obj/extensions/extensions_renderer_resources.ninja includes the mojo_bindings_common__* results. BUG=706786 R=rdevlin.cronin@chromium.org ========== to ========== Fix extensions targets building without chrome The extensions:extensions_renderer_resources_grit target fails to build without building parts of chrome first, meaning that targets in //extensions fail to build. This includes binaries like extensions_unittests, but also anything that depends on extensions/common and not chrome. The dependency was introduced in https://codereview.chromium.org/2727123002 without knowing it would cause build problems. This change seems to ensure the additional necessary mojo files are generated before being used, without introducing additional major chrome dependencies on top of those already here. Specifically, it ensures that obj/extensions/extensions_renderer_resources.ninja includes the mojo_bindings_common__* results. BUG=706786 R=rdevlin.cronin@chromium.org ==========
michaelpg@chromium.org changed required reviewers: + rdevlin.cronin@chromium.org
michaelpg@chromium.org changed reviewers: + takumif@chromium.org
michaelpg@chromium.org changed required reviewers: + rdevlin.cronin@chromium.org
Not thrilled about this, but really it's just updating the deps to reflect the current dependencies that are in the code, so it should be okay (though ideally fixed with moving all those mojo files out of chrome!) LGTM https://codereview.chromium.org/2809303002/diff/1/extensions/BUILD.gn File extensions/BUILD.gn (right): https://codereview.chromium.org/2809303002/diff/1/extensions/BUILD.gn#newcode55 extensions/BUILD.gn:55: "//chrome/browser/media/router:mojo_bindings_common", Does mojo_bindings_common__generator work?
lgtm. I'm currently working on a patch to remove the dependency from extensions/.
https://codereview.chromium.org/2809303002/diff/1/extensions/BUILD.gn File extensions/BUILD.gn (right): https://codereview.chromium.org/2809303002/diff/1/extensions/BUILD.gn#newcode55 extensions/BUILD.gn:55: "//chrome/browser/media/router:mojo_bindings_common", On 2017/04/11 23:02:56, Devlin wrote: > Does mojo_bindings_common__generator work? It does! It only adds two .stamp files to the generated build line. What I had adds a LOT more. Some day I'll learn what STAMP does. and then how these bindings are generated.
The CQ bit was checked by michaelpg@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from takumif@chromium.org, rdevlin.cronin@chromium.org Link to the patchset: https://codereview.chromium.org/2809303002/#ps20001 (title: "generator")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/04/11 23:24:53, takumif wrote: > lgtm. I'm currently working on a patch to remove the dependency from > extensions/. thanks! I'm also going through //extensions to remove existing dependency problems, so that eventually we can have gn check running in the presubmit.
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491955434173370,
"parent_rev": "7e5e14e80e181ad914e09d9f607281f248ec49e6", "commit_rev":
"01d3aa8fc776bb4c2ca45d8e77e1f120e7d61f1b"}
Message was sent while issue was closed.
Description was changed from ========== Fix extensions targets building without chrome The extensions:extensions_renderer_resources_grit target fails to build without building parts of chrome first, meaning that targets in //extensions fail to build. This includes binaries like extensions_unittests, but also anything that depends on extensions/common and not chrome. The dependency was introduced in https://codereview.chromium.org/2727123002 without knowing it would cause build problems. This change seems to ensure the additional necessary mojo files are generated before being used, without introducing additional major chrome dependencies on top of those already here. Specifically, it ensures that obj/extensions/extensions_renderer_resources.ninja includes the mojo_bindings_common__* results. BUG=706786 R=rdevlin.cronin@chromium.org ========== to ========== Fix extensions targets building without chrome The extensions:extensions_renderer_resources_grit target fails to build without building parts of chrome first, meaning that targets in //extensions fail to build. This includes binaries like extensions_unittests, but also anything that depends on extensions/common and not chrome. The dependency was introduced in https://codereview.chromium.org/2727123002 without knowing it would cause build problems. This change seems to ensure the additional necessary mojo files are generated before being used, without introducing additional major chrome dependencies on top of those already here. Specifically, it ensures that obj/extensions/extensions_renderer_resources.ninja includes the mojo_bindings_common__* results. BUG=706786 R=rdevlin.cronin@chromium.org Review-Url: https://codereview.chromium.org/2809303002 Cr-Commit-Position: refs/heads/master@{#463870} Committed: https://chromium.googlesource.com/chromium/src/+/01d3aa8fc776bb4c2ca45d8e77e1... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/01d3aa8fc776bb4c2ca45d8e77e1... |
