Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1121)

Unified Diff: chrome/installer/util/BUILD.gn

Issue 1545803002: Use .rc strings in fewer places for installer util. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/installer/test/BUILD.gn ('k') | chrome/tools/crash_service/BUILD.gn » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/installer/util/BUILD.gn
diff --git a/chrome/installer/util/BUILD.gn b/chrome/installer/util/BUILD.gn
index 4cd7da4e3a0c995acaf52f18af74d696b30d67f8..2aaea9f973272c0edcde3e2a08c55ab8715cba4b 100644
--- a/chrome/installer/util/BUILD.gn
+++ b/chrome/installer/util/BUILD.gn
@@ -5,16 +5,12 @@
import("//build/config/chrome_build.gni")
import("//testing/test.gni")
-# Generally you should depend on this target, see below for discussion.
-group("util") {
- public_deps = [
- ":with_no_strings_some_things_wont_work",
- ]
- if (is_win) {
- public_deps += [ ":strings" ]
- }
-}
-
+# This file deliberately has no default "util" target so dependants have to
+# specify with the ":with_no_strings" or ":with_rc_strings" variants. Random
+# code that ends up getting linked into chrome proper should depend on the
+# ":with_no_strings" variant. Other standalone apps will need to decide if they
+# need the resource strings or not.
+#
# chrome/installer/util has generated strings on Windows. These appear as
# Windows resources and are fairly large (~200KB). They are generated by the
# ":generate_strings" target and compiled by the ":strings" target.
@@ -25,18 +21,24 @@ group("util") {
# resource had to be manually linked, so the strings never ended up being in
# chrome.exe and everything was fine.
#
-# However, this is obviously a fragile and confusing situation. If you depend
-# on this target rather than going through the ":util" version that links the
-# strings above, anything that uses localized strings won't work. There is
-# no definition of what works and doesn't work, and this may also change over
-# time. As an example at the time of this writing, chrome.exe calls
-# BrowserDistribution::GetRegistryPath. This function doesn't use any strings,
-# but lots of other functions in BrowserDistribution do use localized strings.
+# Other code, like chrome.dll links to installer util, and hooks up a
+# TranslationDelegate which overrides the strings retrieved from the resources
+# with grit pak strings.
+#
+# In both of these cases, link to the ":with_no_strings" variant. However, this
+# is obviously a fragile and confusing situation. In the "I don't use strings
+# at all case", there is no definition of what works and doesn't work, and this
+# may also change over time. As an example at the time of this writing,
+# chrome.exe calls BrowserDistribution::GetRegistryPath. This function doesn't
+# use any strings, but lots of other functions in BrowserDistribution do use
+# localized strings.
#
# Ideally, this should be cleaved in two parts: the main "installer util" and
# some kind of mini installer util that just contains the functions needed by
-# chrome.exe and any other clients that don't need the strings.
-static_library("with_no_strings_some_things_wont_work") {
+# chrome.exe and any other clients that don't need the strings. It's likely
+# we would still need the variant with no strings for when chrome.dll replaces
+# all strings with its own versions.
+static_library("with_no_strings") {
deps = [
"//base",
"//chrome:strings",
@@ -221,10 +223,21 @@ static_library("with_no_strings_some_things_wont_work") {
}
}
+# Use this version of installer_util to link to the generated strings in .rc
+# format.
+group("with_rc_strings") {
+ public_deps = [
+ ":with_no_strings",
+ ]
+ if (is_win) {
+ public_deps += [ ":strings" ]
+ }
+}
+
action("generate_strings") {
visibility = [
":strings",
- ":with_no_strings_some_things_wont_work",
+ ":with_no_strings",
]
script = "prebuild/create_string_rc.py"
@@ -312,8 +325,7 @@ if (is_win) {
]
deps = [
- ":strings",
- ":util",
+ ":with_rc_strings",
"//base",
"//base:i18n",
"//base/test:test_support",
« no previous file with comments | « chrome/installer/test/BUILD.gn ('k') | chrome/tools/crash_service/BUILD.gn » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698