win: Don't use rc.exe's built-in preprocessor.

Microsoft rc.exe has its built-in preprocessor that has a limit of 32
characters for macro names.

We already use clang-cl's preprocessor for our own rc that we use in cross
builds. That works well, so feed the output from that into Microsoft
rc.exe too.

This allows removing some workarounds for BUILDFLAG() use in .rc files and
shouldn't change behavior.

Bug: 961769
Change-Id: I21ea2e62323a27c61f8c79c267814ed2f522eded
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860097
Commit-Queue: Nico Weber <thakis@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Auto-Submit: Nico Weber <thakis@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705630}

build/toolchain/win/rc/rc.py

@@ -162,7 +162,7 @@ def RunRc(preprocessed_output, is_utf8, flags):
   rc_cmd.append('/fo' + flags.output)
   if is_utf8:
     rc_cmd.append('/utf-8')
-  # TODO(thakis): rc currently always prints full paths for /showIncludes,
+  # TODO(thakis): cl currently always prints full paths for /showIncludes,
   # but clang-cl /P doesn't.  Which one is right?
   if flags.show_includes:
     rc_cmd.append('/showIncludes')
@@ -172,9 +172,65 @@ def RunRc(preprocessed_output, is_utf8, flags):
   rc_cmd += flags.includes
   p = subprocess.Popen(rc_cmd, stdin=subprocess.PIPE)
   p.communicate(input=preprocessed_output)
+
+  if flags.show_includes and p.returncode == 0:
+    # Since tool("rc") can't have deps, add deps on this script and on rc.py
+    # and its deps here, so that rc edges become dirty if rc.py changes.
+    print('Note: including file: ../../build/toolchain/win/tool_wrapper.py')
+    print('Note: including file: ../../build/toolchain/win/rc/rc.py')
+    print(
+        'Note: including file: ../../build/toolchain/win/rc/linux64/rc.sha1')
+    print('Note: including file: ../../build/toolchain/win/rc/mac/rc.sha1')
+    print(
+        'Note: including file: ../../build/toolchain/win/rc/win/rc.exe.sha1')
+
   return p.returncode
 
 
+def CompareToMsRcOutput(preprocessed_output, is_utf8, flags):
+  msrc_in = flags.output + '.preprocessed.rc'
+
+  # Strip preprocessor line markers.
+  preprocessed_output = re.sub(r'^#.*$', '', preprocessed_output, flags=re.M)
+  if is_utf8:
+    preprocessed_output = preprocessed_output.decode('utf-8').encode('utf-16le')
+  with open(msrc_in, 'wb') as f:
+    f.write(preprocessed_output)
+
+  msrc_out = flags.output + '_ms_rc'
+  msrc_cmd = ['rc', '/nologo', '/fo' + msrc_out]
+
+  # Make sure rc-relative resources can be found. rc.exe looks for external
+  # resource files next to the file, but the preprocessed file isn't where the
+  # input was.
+  # Note that rc searches external resource files in the order of
+  # 1. next to the input file
+  # 2. relative to cwd
+  # 3. next to -I directories
+  # Changing the cwd means we'd have to rewrite all -I flags, so just add
+  # the input file dir as -I flag. That technically gets the order of 1 and 2
+  # wrong, but in Chromium's build the cwd is the gn out dir, and generated
+  # files there are in obj/ and gen/, so this difference doesn't matter in
+  # practice.
+  if os.path.dirname(flags.input):
+    msrc_cmd += [ '-I' + os.path.dirname(flags.input) ]
+
+  # Microsoft rc.exe searches for referenced files relative to -I flags in
+  # addition to the pwd, so -I flags need to be passed both to both
+  # the preprocessor and rc.
+  msrc_cmd += flags.includes
+
+  # Input must come last.
+  msrc_cmd += [ msrc_in ]
+
+  rc_exe_exit_code = subprocess.call(msrc_cmd)
+  # Assert Microsoft rc.exe and rc.py produced identical .res files.
+  if rc_exe_exit_code == 0:
+    import filecmp
+    assert filecmp.cmp(msrc_out, flags.output)
+  return rc_exe_exit_code
+
+
 def main():
   # This driver has to do these things:
   # 1. Parse flags.
@@ -185,7 +241,17 @@ def main():
   flags = ParseFlags()
   rc_file_data, is_utf8 = ReadInput(flags.input)
   preprocessed_output = Preprocess(rc_file_data, flags)
-  return RunRc(preprocessed_output, is_utf8, flags)
+  rc_exe_exit_code = RunRc(preprocessed_output, is_utf8, flags)
+
+  # 5. On Windows, we also call Microsoft's rc.exe and check that we produced
+  #   the same output.
+  # Since Microsoft's rc has a preprocessor that only accepts 32 characters
+  # for macro names, feed the clang-preprocessed source into it instead
+  # of using ms rc's preprocessor.
+  if sys.platform == 'win32' and rc_exe_exit_code == 0:
+    rc_exe_exit_code = CompareToMsRcOutput(preprocessed_output, is_utf8, flags)
+
+  return rc_exe_exit_code
 
 
 if __name__ == '__main__':

build/toolchain/win/tool_wrapper.py

@@ -212,46 +212,11 @@ class WinTool(object):
   def ExecRcWrapper(self, arch, *args):
     """Converts .rc files to .res files."""
     env = self._GetEnv(arch)
-
-    # We run two resource compilers:
-    # 1. A custom one at build/toolchain/win/rc/rc.py which can run on
-    #    non-Windows, and which has /showIncludes support so we can track
-    #    dependencies (e.g. on .ico files) of .rc files.
-    # 2. On Windows, regular Microsoft rc.exe, to make sure rc.py produces
-    #    bitwise identical output.
-
-    # 1. Run our rc.py.
-    # Also pass /showIncludes to track dependencies of .rc files.
     args = list(args)
     rcpy_args = args[:]
     rcpy_args[0:1] = [sys.executable, os.path.join(BASE_DIR, 'rc', 'rc.py')]
-    rcpy_res_output = rcpy_args[-2]
-    assert rcpy_res_output.startswith('/fo')
-    assert rcpy_res_output.endswith('.res')
-    rc_res_output = rcpy_res_output + '_ms_rc'
-    args[-2] = rc_res_output
     rcpy_args.append('/showIncludes')
-    rc_exe_exit_code = subprocess.call(rcpy_args, env=env)
-    if rc_exe_exit_code == 0:
-      # Since tool("rc") can't have deps, add deps on this script and on rc.py
-      # and its deps here, so that rc edges become dirty if rc.py changes.
-      print('Note: including file: ../../build/toolchain/win/tool_wrapper.py')
-      print('Note: including file: ../../build/toolchain/win/rc/rc.py')
-      print(
-          'Note: including file: ../../build/toolchain/win/rc/linux64/rc.sha1')
-      print('Note: including file: ../../build/toolchain/win/rc/mac/rc.sha1')
-      print(
-          'Note: including file: ../../build/toolchain/win/rc/win/rc.exe.sha1')
-
-    # 2. Run Microsoft rc.exe.
-    if sys.platform == 'win32' and rc_exe_exit_code == 0:
-      rc_exe_exit_code = subprocess.call(args, shell=True, env=env)
-      # Assert Microsoft rc.exe and rc.py produced identical .res files.
-      if rc_exe_exit_code == 0:
-        import filecmp
-        # Strip "/fo" prefix.
-        assert filecmp.cmp(rc_res_output[3:], rcpy_res_output[3:])
-    return rc_exe_exit_code
+    return subprocess.call(rcpy_args, env=env)
 
   def ExecActionWrapper(self, arch, rspfile, *dirname):
     """Runs an action command line from a response file using the environment

chrome/app/BUILD.gn

@@ -7,7 +7,6 @@ import("//chrome/common/features.gni")
 import("//components/gwp_asan/buildflags/buildflags.gni")
 import("//components/nacl/features.gni")
 import("//ppapi/buildflags/buildflags.gni")
-import("//printing/buildflags/buildflags.gni")
 import("//tools/grit/grit_rule.gni")
 import("//tools/ipc_fuzzer/ipc_fuzzer.gni")
 
@@ -50,12 +49,6 @@ source_set("chrome_dll_resources") {
 
   if (is_win) {
     sources += [ "chrome_dll.rc" ]
-    if (enable_basic_printing) {
-      # The resource compiler can only handle macro functions up to 31 chars
-      # which the buildflag system produces for this. Make a define so we can
-      # toggle off of the enable-basic-printing flag in the .rc file.
-      defines = [ "ENABLE_PRINTING_FOR_RC" ]
-    }
     deps += [ "//printing/buildflags" ]
   }
 }

chrome/app/chrome_dll.rc

@@ -2,6 +2,7 @@
 //
 #include "chrome_dll_resource.h"
 #include "chrome_command_ids.h"
+#include "printing/buildflags/buildflags.h"
 
 #define APSTUDIO_READONLY_SYMBOLS
 /////////////////////////////////////////////////////////////////////////////
@@ -35,10 +36,7 @@ IDR_MAINFRAME ACCELERATORS
 BEGIN
     VK_BACK,        IDC_BACK,                   VIRTKEY
     VK_LEFT,        IDC_BACK,                   VIRTKEY, ALT
-// This really should be using the buildflag but the resource compiler fails on
-// macro functions longer than 31 chars which this command produces. The BUILD
-// sets this regular define for this target only for this line to consume.
-#ifdef ENABLE_PRINTING_FOR_RC  // #if BUILDFLAG(ENABLE_PRINTING)
+#if BUILDFLAG(ENABLE_PRINTING)
     "P",            IDC_BASIC_PRINT,            VIRTKEY, CONTROL, SHIFT
 #endif  // ENABLE_PRINTING_FOR_RC
     "D",            IDC_BOOKMARK_ALL_TABS,      VIRTKEY, CONTROL, SHIFT

chrome/installer/setup/BUILD.gn

@@ -10,8 +10,6 @@ import("//testing/test.gni")
 
 buildflag_header("buildflags") {
   header = "buildflags.h"
-
-  # Use ZUCCHINI since ZUCCHINI_ENABLED is too long a name for setup.rc.
   flags = [ "ZUCCHINI=$use_zucchini" ]
 }
 
@@ -26,13 +24,6 @@ if (is_win) {
       "uninstall.h",
     ]
 
-    if (is_chrome_branded) {
-      # The resource compiler can only handle macro functions up to 31 chars
-      # which the buildflag system produces for this. Make a define so we can
-      # toggle off of the enable-basic-printing flag in the .rc file.
-      defines = [ "GOOGLE_CHROME_BRANDING_FOR_RC" ]
-    }
-
     configs -= [ "//build/config/win:console" ]
     configs += [ "//build/config/win:windowed" ]
 

chrome/installer/setup/setup.rc

@@ -14,6 +14,7 @@
 /////////////////////////////////////////////////////////////////////////////
 #undef APSTUDIO_READONLY_SYMBOLS
 
+#include "build/branding_buildflags.h"
 #include "chrome/installer/setup/buildflags.h"
 
 /////////////////////////////////////////////////////////////////////////////
@@ -58,10 +59,7 @@ IDI_SETUP               ICON                    "setup.ico"
 #endif    // English (U.S.) resources
 /////////////////////////////////////////////////////////////////////////////
 
-// This really should be using the buildflag but the resource compiler fails on
-// macro functions longer than 31 chars which this command produces. The BUILD
-// sets this regular define for this target only for this line to consume.
-#ifdef GOOGLE_CHROME_BRANDING_FOR_RC
+#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
 
 /////////////////////////////////////////////////////////////////////////////
 //
@@ -118,7 +116,7 @@ IDR_OEMPG_VI.HTML       HTML   "eula\\oem_vi.html"
 IDR_OEMPG_ZH_CN.HTML    HTML   "eula\\oem_zh-CN.html"
 IDR_OEMPG_ZH_TW.HTML    HTML   "eula\\oem_zh-TW.html"
 
-#endif  // GOOGLE_CHROME_BRANDING_FOR_RC
+#endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
 
 #ifndef APSTUDIO_INVOKED
 /////////////////////////////////////////////////////////////////////////////