New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 801308 link

Starred by 1 user
Status: Fixed
Owner:
█ roc...@chromium.org
please use my google.com address
Closed: Jan 2018
Cc:
█ yzshen@chromium.org
█ rdsmith@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Mojo: Typemap public_headers should not add to target sources

Project Member Reported by roc...@chromium.org, Jan 11 2018 
Listing headers in a typemap's public_headers field not only includes those headers in generated mojom.h files, but also adds the headers to the |sources| list of the corresponding build target.

I may have added this at some point to work around some kind of generated file dependency / build ordering issue, but I'll revisit it and find a way to undo it, as it is IMO clearly incorrect.

Comment 1 by roc...@chromium.org, Jan 11 2018 

traits_headers is also treated incorrectly here for the same reason, given that it's possible to depend on traits owned by another target. traits_headers which belong to the mojom target should therefore also be explicitly listed in the typemap's |sources|.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 17 2018 

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

commit db79117e3ef1721a55fec4d2a60c30054b8f5bce
Author: Ken Rockot <rockot@chromium.org>
Date: Wed Jan 17 20:36:08 2018

Fix mojom typemap dependencies

public_headers and traits_headers entries were incorrectly being slurped
into generated C++ bindings targets' |sources| lists. This is not correct
and caused GN to miss a whole bunch of legitimately missing dependencies.

Fixes the mojom GN rule behavior and updates affected targets.

This change also for some reason (I'm assuming breakage of obscure
transitive dependencies) started triggering a few other non-mojom
targets to fail gn check for legitimate reasons, so those issues have
been corrected as well.

Bug:  801308 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ia6e70f9b7c9bf969ef05054d32928d44d6171971

TBR=danakj@chromium.org,sky@chromium.org,mfoltz@chromium.org,bradnelson@chromium.org,nparker@chromium.org,miu@chromium.org

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ia6e70f9b7c9bf969ef05054d32928d44d6171971
Reviewed-on: https://chromium-review.googlesource.com/862702
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529864}
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/cc/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/chrome/browser/media/router/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/chrome/browser/vr/elements/audio_permission_prompt_texture.cc
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/chrome/browser/vr/ui_scene_constants.h
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/chrome/browser/vr/ui_scene_creator.cc
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/chrome/common/media_router/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/chrome/common/media_router/mojo/media_router.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/chrome/utility/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/components/nacl/renderer/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/components/safe_browsing/renderer/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/content/browser/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/content/public/common/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/content/public/common/manifest.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/extensions/browser/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/media/capture/mojo/video_capture_types.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/mojo/public/cpp/bindings/README.md
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/mojo/public/cpp/bindings/tests/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/preferences/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/preferences/public/cpp/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/coordination_unit.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/coordination_unit_id.h
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/coordination_unit_struct_traits.h
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/memory_instrumentation/global_memory_dump.h
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation_struct_traits.h
[add] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/cpp/resource_coordinator_base_export.h
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/resource_coordinator/public/interfaces/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/ui/public/interfaces/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/ui/public/interfaces/cursor/cursor.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/ui/public/interfaces/ime/ime.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/viz/public/cpp/compositing/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/services/viz/public/cpp/compositing/transferable_resource.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/ui/display/mojo/display.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/ui/events/devices/mojo/input_device.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/ui/gfx/mojo/ca_layer_params.typemap
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/ui/ozone/public/interfaces/BUILD.gn
[modify] https://crrev.com/db79117e3ef1721a55fec4d2a60c30054b8f5bce/ui/ozone/public/interfaces/overlay_surface_candidate.typemap

Comment 3 by roc...@chromium.org, Jan 17 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment