Issue metadata
Sign in to add a comment
|
ax_enums.h build flake |
||||||||||||||||||||||||
Issue descriptionseems that some mixture of hard_dependency, dependencies, export_dependent_settings, etc. is missing: [4958/33267] CXX obj\ui\app_list\presenter\test\app_list_presenter_test_support.app_list_presenter_impl_test_api.obj FAILED: obj/ui/app_list/presenter/test/app_list_presenter_test_support.app_list_presenter_impl_test_api.obj ninja -t msvc -e environment.x86 -- C:\b\build\slave\cache\cipd\goma/gomacc "C:\b\depot_tools\win_toolchain\vs_files\95ddda401ec5678f15eeed01d2bee08fcbc5ee97\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC @obj\ui\app_list\presenter\test\app_list_presenter_test_support.app_list_presenter_impl_test_api.obj.rsp /c ..\..\ui\app_list\presenter\test\app_list_presenter_impl_test_api.cc /Foobj\ui\app_list\presenter\test\app_list_presenter_test_support.app_list_presenter_impl_test_api.obj /Fdobj\ui\app_list\presenter\app_list_presenter_test_support.cc.pdb c:\b\build\slave\win\build\src\ui\views\view.h(22): fatal error C1083: Cannot open include file: 'ui/accessibility/ax_enums.h': No such file or directory https://build.chromium.org/p/chromium/builders/Win/builds/43375
,
May 16 2016
I think the problem here is that both export_dependent_settings and hard_dependency is required in all targets that have a public header including ax_enums.h Since views/view.h #inclues ax_enum.h, to fix this "nicely", views.gyp:views would need to be a hard_dependency for everything that depends on it. That's perhaps overkill (e.g. it might reduce build parallelism). The "easy" fix: just have app_list_presenter.gyp depend on ax_gen The *best* fix, I think is to have ax_enums.idl generate `enum class` rather than merely `enum`. Then view.h can forward-declare ui::AXEvent and not have to include ax_enums.h at all.
,
May 17 2016
Issue 510339 has been merged into this issue.
,
May 17 2016
This also came up in Issue 536641 The ideal fix (i.e. forward-declare an enum class) is in Issue 612382 , but it needs time to cook. Compiler flakes are pretty annoying, so I've made https://codereview.chromium.org/1984153002 to hopefully make this current collection of flakes go away.
,
May 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/93fa270c60338957b15cefb20e5553fb67cb2ba3 commit 93fa270c60338957b15cefb20e5553fb67cb2ba3 Author: tapted <tapted@chromium.org> Date: Tue May 17 07:25:32 2016 Workaround for flaky ax_enums.h errors when compiling under app_list_presenter ui/views/view.h #includes ax_enums.h and app_list_presenter #includes view.h. This means ax_enums.h needs to exist before anything in app_list_presenter is compiled, even though app_list_presenter doesn't use anything from ax_enums.h. An alternative might be to make views.gyp:views a hard_dependency of everything that depends on it. However, it seems unfair to force that, since it's really only ax_gen that is the "hard" dependency, and it's already correctly marked as a hard_dependency. Exploring a nicer fix in http://crbug.com/612382 , which involves view.h simply not #including ax_enums.h BUG= 611898 Review-Url: https://codereview.chromium.org/1984153002 Cr-Commit-Position: refs/heads/master@{#394074} [modify] https://crrev.com/93fa270c60338957b15cefb20e5553fb67cb2ba3/ui/app_list/presenter/BUILD.gn [modify] https://crrev.com/93fa270c60338957b15cefb20e5553fb67cb2ba3/ui/app_list/presenter/app_list_presenter.gyp
,
May 18 2016
cycle times are slow. But no new flakes since https://build.chromium.org/p/chromium/builders/Win/builds/43456 which was at r394023; before #c5 landed. The latest flake seems to be another mojo one in https://build.chromium.org/p/chromium/builders/Win/builds/43482 FAILED: obj/chrome/browser/media/router/mojo/media_router_test_support.media_router_mojo_test.obj ninja -t msvc -e environment.x86 -- C:\b\build\slave\cache\cipd\goma/gomacc "C:\b\depot_tools\win_toolchain\vs_files\95ddda401ec5678f15eeed01d2bee08fcbc5ee97\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC @obj\chrome\browser\media\router\mojo\media_router_test_support.media_router_mojo_test.obj.rsp /c ..\..\chrome\browser\media\router\mojo\media_router_mojo_test.cc /Foobj\chrome\browser\media\router\mojo\media_router_test_support.media_router_mojo_test.obj /Fdobj\chrome\browser\media\router\media_router_test_support.cc.pdb c:\b\build\slave\win\build\src\services\shell\public\cpp\connect.h(10): fatalerror C1083: Cannot open include file: 'services/shell/public/interfaces/interface_provider.mojom.h': No such file or directory moving to p2 and leaving open until I address the TODOs in #c5
,
May 19 2016
Latest failure at https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/19732
,
May 20 2016
The flake in #c7 is hopefully addressed by https://codereview.chromium.org/1997853002/ (landed in r394888 -> https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/19742). Things are green since then. There's progress in Issue 612382 as well. And http://crrev.com/1988613002 should resolve/isolate/prevent the particular flakes that ocurred in the initial report and in #c7.
,
May 26 2016
Ugh. The app_list_presenter flake happened again at r395937 in https://build.chromium.org/p/chromium/builders/Win%20x64/builds/1001/ this time due to test targets. Workaroundy fix: https://codereview.chromium.org/2012773003/ There's also news in Issue 612382 -- exploring a way to fix this "faster" by enforcing enums to be int-sized rather than enum classes. This could get the resolution out with a lot less churn. See https://codereview.chromium.org/1999633002/ for how that could look. But it's also worth noting https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/cookbook.md#hard_dependency It says GYP code sometimes sets 'hard_dependency': 1, to indicate that the current target must be build before its dependents. GN can deduce this internally, so you can ignore this directive. So maybe the GN migration will also resolve the flakes (hooray!). But there may still be a lesser concern about complicated chains of hard dependencies creating bottlenecks in the build. There was also a surprising number of things wanting an ex_enums.h #include for IWYU -- https://codereview.chromium.org/1996893002/ Also one header where the way to forward declare wasn't clear - https://codereview.chromium.org/1999633002/diff/100001/content/public/common/common_param_traits_macros.h . It raises the possibility that addressing these flakes purely by forward-declaring may be a dead end :(
,
May 26 2016
For the last bit, you could have just manually defined FOO_LAST values in a regular header, and then static_assert that these are the same as the autogenerated _LAST fields in some cc file.
,
May 26 2016
Probably not surprising, but I like https://codereview.chromium.org/1999633002/ :-)
,
May 26 2016
GYP is getting more rare so I'm not personally concerned about having too many hard deps in GYP these days.
,
May 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f1bce9d0653131ac106bd31550b65e6672ecc1b8 commit f1bce9d0653131ac106bd31550b65e6672ecc1b8 Author: tapted <tapted@chromium.org> Date: Thu May 26 08:45:11 2016 Add ax_gen dependency to app_list_presenter test targets These targets all have files that #include view.h, which #includes ax_enums.h. Therefore, building these targets can not begin until ax_enums.h has been generated. Otherwise the compile can fail. The dependency was added to the non-test app_list_presenter target in r394074. That helped, but it didn't do enough. Example: https://build.chromium.org/p/chromium/builders/Win%20x64/builds/1001/ BUG= 611898 Review-Url: https://codereview.chromium.org/2012773003 Cr-Commit-Position: refs/heads/master@{#396150} [modify] https://crrev.com/f1bce9d0653131ac106bd31550b65e6672ecc1b8/ui/app_list/presenter/BUILD.gn [modify] https://crrev.com/f1bce9d0653131ac106bd31550b65e6672ecc1b8/ui/app_list/presenter/app_list_presenter.gyp
,
Jan 27 2017
I think GN has resolved this for the most part. There's still Issue 612382 , which I think still makes sense, but there were some use cases that meant it could never fully avoid targets requiring a transitive dependency on ax_enums.h. (So it's not useful for this Issue). |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by michae...@chromium.org
, May 13 2016