Copy VC CRT DLLs for component build even when VS is not installed.

chrome/tools/build/win/create_installer_archive.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Script to create Chrome Installer archive.
 
   This script is used to create an archive of all the files required for a
   Chrome install in appropriate directory structure. It reads chrome.release
   file as input, creates chrome.7z archive, compresses setup.exe and
@@ skipping 348 matching lines @@
         "C:/Program Files (x86)/Microsoft Visual Studio */VC/redist/"
         "Debug_NonRedist/x86/Microsoft.*.DebugCRT/*.dll")
   elif build_dir.endswith('Release/'):
     crt_dlls = glob.glob(
         "C:/Program Files (x86)/Microsoft Visual Studio */VC/redist/x86/"
         "Microsoft.*.CRT/*.dll")
   else:
     print ("Warning: CRT DLLs not copied, could not determine build "
            "configuration from output directory.")
 
+  # Also handle the case where someone is building using only winsdk and
+  # doesn't have Visual Studio installed.
+  if not crt_dlls:

[gab, 2012/05/20 14:42:54]

Now that this is getting bigger I would extract all of the CRT DLL copying in a
separate function above and just call it from here.

[dmazzoni, 2012/05/21 21:28:39]

Done.

+    if os.access("C:/Windows/SysWOW64", os.F_OK):
+      sys_dll_dir = "C:/Windows/SysWOW64"

[gab, 2012/05/20 14:42:54]

Aren't these the 64-bit DLLs? Chrome is always 32-bit no?

[robertshield, 2012/05/20 17:36:16]

Wow64 means "Windows 32-bit on Windows 64 bit", so this directory contains 32
bit dlls.

[dmazzoni, 2012/05/21 21:28:39]

Worth a comment; I certainly wouldn't have guessed this.

+    else:
+      sys_dll_dir = "C:/Windows/System32"

[gab, 2012/05/20 15:05:01]

These DLLs are found all over the place. I'm not convinced the DLLs in System32
are the ones we want.

Maybe msbuild\... has them somewhere? If so, maybe we want to only copy those
instead of the ones from VS as VS uses msbuild underneath anyways...?

robertshield@ would probably know more.

[robertshield, 2012/05/20 17:36:16]

I don't know offhand which dlls are picked up when using msbuild without VS
installed. I suspect they are the ones in the system dir but would have to
check. If they are then this is fine.

[dmazzoni, 2012/05/21 21:28:39]

FWIW, I searched my system and did not find them anywhere else. Unless I missed
something, winsdk doesn't provide an equivalent to the "redist" directory with
all of the needed runtime DLLs, but I think it does install them to your system
directory if they're not already there.

One possible reason is that winsdk is compatible with multiple versions of VS
(even though it includes one compiler toolchain), while each VS install has
exactly one set of runtime DLLs.

[robertshield, 2012/05/22 02:39:58]

Ok, interesting. Since all that really matters is that the runtime DLLs being
copied are the same as the ones that are built against, this looks fine to me.

+
+    if build_dir.endswith('Debug/'):
+      crt_dlls = glob.glob("%s/msvc*0d.dll" % sys_dll_dir)

[gab, 2012/05/20 14:42:54]

I'd do this inside the "if build_dir.endswith('Debug/'):" of the previous
section (`if not crt_dlls` only of course), this duplicates "if not crt_dlls"
which is a fast check, but avoids duplicating ".endswith()" check which is
slower.

Subsequently I suggest we remove the warning in the "else" section above and
replace it by a warning "if not crt_dlls" still at the end of all of this.

[dmazzoni, 2012/05/21 21:28:39]

I'd argue that the "if os.access" is by far the most expensive check. Since
having VS installed is the more common case, I think it makes sense to defer
that computation to the case where the DLLs weren't found in the obvious place.

Instead, I moved the build_dir.endswith computation to the top so that it's only
done once. Does that work?
I split it into two separate warnings, take a look.

I thought it'd be simpler to default to Release dlls if the build type couldn't
be determined. My reasoning is that if someone's copying the target to another
machine, it's far more likely they're copying a Release build. It couldn't hurt.

[gab, 2012/05/22 15:44:46]

Yep.
Perfect.

+    elif build_dir.endswith('Release/'):
+      crt_dlls = glob.glob("%s/msvc*0.dll" % sys_dll_dir)

[gab, 2012/05/20 15:05:01]

.format() has been preferred over % for string formatting in this function.

This has already been discussed here:
https://chromiumcodereview.appspot.com/9701050/diff/15002/chrome/tools/build/...

[dmazzoni, 2012/05/21 21:28:39]

Actually in this case a simple string append might be the most readable. Does
that look okay? If not I'm happy to use the new .format() awesomeness.

[gab, 2012/05/22 15:44:46]

Works for me.

+
   for dll in crt_dlls:
     shutil.copy(dll, build_dir)
 
   # Copy all the DLLs in |build_dir| to the version directory. Simultaneously
   # build a list of their names to mark them as dependencies of chrome.exe and
   # setup.exe later.
   dlls = glob.glob(os.path.join(build_dir, '*.dll'))
   dll_names = []
   for dll in dlls:
     shutil.copy(dll, version_dir)
@@ skipping 172 matching lines @@
   if not options.resource_file_path:
     options.resource_file_path = os.path.join(options.build_dir,
                                               MINI_INSTALLER_INPUT_FILE)
 
   return options
 
 
 if '__main__' == __name__:
   print sys.argv
   sys.exit(main(_ParseOptions()))