From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Thu, 20 Sep 2018 17:48:59 -0700 Subject: resource_file_conflict.patch Resolve conflict between //chrome's .pak files and //electron's. The paths that chrome code hardcodes require that we generate resources at these paths, but GN throws errors if there are multiple targets that generate the same files. This is due to the hardcoded names here: https://chromium.googlesource.com/chromium/src/+/69.0.3497.106/ui/base/resource/resource_bundle.cc#780 and here: https://chromium.googlesource.com/chromium/src/+/69.0.3497.106/ui/base/resource/resource_bundle_mac.mm#50 This isn't needed on Mac because resource files are copied into the app bundle, and are built in `$root_out_dir/electron_repack` (while Chromium's resources target `$root_out_dir/repack`), but on Windows and Linux, the resource files go directly in `$root_out_dir`, and so they conflict. We don't actually ever generate Chromium's resource paks, but without this patch, GN refuses to generate the ninja files: ERROR at //tools/grit/repack.gni:35:3: Duplicate output file. action(_repack_target_name) { ^---------------------------- Two or more targets generate the same output: chrome_100_percent.pak This is can often be fixed by changing one of the target names, or by setting an output_name on one of them. Collisions: //chrome:packed_resources_100_percent //electron:packed_resources_100_percent See //tools/grit/repack.gni:35:3: Collision. action(_repack_target_name) { ^---------------------------- Some alternatives to this patch: 1. Refactor upstream in such a way that the "chrome" pak names were configurable, for instance by adding a method to ResourceBundle::Delegate that LoadChromeResources would check. 2. Pass a Delegate that overrides `GetPathForResourcePack`, check for the `chrome_{100,200}_percent.pak` filenames, and rewrite them to `electron_{100,200}_percent.pak`. 3. Initialize the resource bundle with DO_NOT_LOAD_COMMON_RESOURCES and load the paks ourselves. None of these options seems like a substantial maintainability win over this patch to me (@nornagon). diff --git a/chrome/BUILD.gn b/chrome/BUILD.gn index 4894620e63bda3946292eb43ae19078172860843..b7c6293dfb3177164a34254613b320c690563a37 100644 --- a/chrome/BUILD.gn +++ b/chrome/BUILD.gn @@ -1386,7 +1386,7 @@ if (is_chrome_branded && !is_android) { } } -if (!is_android) { +if (!is_android && !is_electron_build) { chrome_paks("packed_resources") { if (is_mac) { output_dir = "$root_gen_dir/repack" @@ -1406,6 +1406,12 @@ if (!is_android) { } } +if (is_electron_build) { + group("packed_resources") { + public_deps = [ "//electron:packed_resources" ] + } +} + repack("browser_tests_pak") { sources = [ "$root_gen_dir/chrome/webui_test_resources.pak" ] output = "$root_out_dir/browser_tests.pak"