Make chromecast build not use output_dir = "$root_gen_dir/chromecast_strings" |
||
Issue descriptionchromecast/app/BUILD.gn currently sets output_dir = "$root_gen_dir/chromecast_strings" for the chromecast_settings target. That's not ideal: With this output_dir, once we make this target have use_qualified_include=true too, the .h file needs to be included via #include chromecast_strings/grit/chromecast_settings.h" which in turn requires a +chromecast_strings entry in a DEPS file, which in return needs an LGTM from a chromecast_strings/OWNER for the new dep -- but that file doesn't exist. Using the default output dir nicely makes grit outputs match the existing DEPS system. It sounds like there's no great reason for the output_dir override to exist, so let's get rid of it.
,
Mar 14 2017
Looking now.
,
Mar 21 2017
Did this happen?
,
Mar 22 2017
Sorry, looked into this then got pulled aside for something. We're using this directory as a collection point for our custom repack script, which packs our internal and external resources together: https://cs.chromium.org/chromium/src/chromecast/BUILD.gn?rcl=aac0eb99e1c73e71d69b075bc17f5dd84b934a6b&l=412 The file you're concerned about, "chromecast_settings.h", gets pulled in here: https://cs.chromium.org/chromium/src/chromecast/app/BUILD.gn?rcl=96dd5df887ad492d2da843329098474fb8f8d5d6&l=116 This is the only file that's referenced in native code, and I suppose it wouldn't be unreasonable to change the target directory to the default dir (<out>/gen/chromecast/app). I'll give it a whirl.
,
Mar 22 2017
On second thought, I think we can eliminate this script entirely by using a GN template. I'll do it all in the same patch.
,
Mar 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c5624b304dd7710b1422a16b7d4039c0dcef283 commit 5c5624b304dd7710b1422a16b7d4039c0dcef283 Author: slan <slan@chromium.org> Date: Thu Mar 23 20:24:14 2017 [Chromecast] Remove chromecast_repack_locales.py. This script predates GN templates, which allow us to do all of our resource packing in GN. Eliminate the script and correct some bad out directory conventions. BUG= 697090 TEST= diff the .pak output before and after the change <= no diff Review-Url: https://codereview.chromium.org/2773453003 Cr-Commit-Position: refs/heads/master@{#459200} [modify] https://crrev.com/5c5624b304dd7710b1422a16b7d4039c0dcef283/chromecast/BUILD.gn [modify] https://crrev.com/5c5624b304dd7710b1422a16b7d4039c0dcef283/chromecast/app/BUILD.gn [modify] https://crrev.com/5c5624b304dd7710b1422a16b7d4039c0dcef283/chromecast/browser/DEPS [modify] https://crrev.com/5c5624b304dd7710b1422a16b7d4039c0dcef283/chromecast/browser/cast_http_user_agent_settings.cc [delete] https://crrev.com/0bd994d15c644515a66f1ddd081122894e314e3b/chromecast/tools/build/chromecast_repack_locales.py
,
Mar 27 2017
Thanks!
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4f97ba825e075c7660bcb2c4dfc07e2a60d73d48 commit 4f97ba825e075c7660bcb2c4dfc07e2a60d73d48 Author: slan <slan@chromium.org> Date: Thu Mar 30 17:09:30 2017 [Chromecast] Require chromecast OWNERS to be aware of locale changes. Whenever the locale lists declared in //build/config/locales.gni change, someone from the Chromecast team should be added to the CL as an FYI. Enforce this via a python script that checks the locales against a fixed list. BUG= 697090 TEST= modify the locales list, make sure the build complains. Review-Url: https://codereview.chromium.org/2779663003 Cr-Commit-Position: refs/heads/master@{#460803} [modify] https://crrev.com/4f97ba825e075c7660bcb2c4dfc07e2a60d73d48/chromecast/app/BUILD.gn [add] https://crrev.com/4f97ba825e075c7660bcb2c4dfc07e2a60d73d48/chromecast/app/verify_cast_locales.py |
||
►
Sign in to add a comment |
||
Comment 1 by thakis@chromium.org
, Mar 10 2017