Copy local CRT DLLs to gn output directories

Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when 'gn gen'
or 'gn args' runs.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934, 585556, 610336
Committed: https://crrev.com/e7bd0348e692e635c039328cd06aac1ce23b7d4f
Cr-Commit-Position: refs/heads/master@{#397189}

[brucedawson, 2016-05-31 21:36:18]

brucedawson@chromium.org changed reviewers:
+ scottmg@chromium.org

[brucedawson, 2016-05-31 21:36:19]

This should fix mini_installer and isolate builds with
DEPOT_TOOLS_WIN_TOOLCHAIN=0, and generally lets us make assumptions about what
DLLs will be in the output directory.

This should make CopyVisualStudioRuntimeDLLs() in create_installer_archive.py
obsolete - I'm testing that, but PTAL.

[scottmg, 2016-05-31 21:57:45]

https://codereview.chromium.org/2021353003/diff/20001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/2021353003/diff/20001/build/vs_toolchain.py#n...
build/vs_toolchain.py:84: vs_runtime_dll_dirs = [ 'C:/Windows/System32',
'C:/Windows/SysWOW64' ]
Hmm, I'm confused. I would have thought it was x86, x64.

But looking below at CopyVsRuntimeDlls() I see it's x86,x64, but CopyDlls() is
x64,x86! Do you know what's going on?

https://codereview.chromium.org/2021353003/diff/20001/build/vs_toolchain.py#n...
build/vs_toolchain.py:208: ucrt_src_dir = os.path.join(source_dir, 'downlevel',
'api-ms-win-*.dll')
Maybe ucrt_src_pattern or _glob instead.

[brucedawson, 2016-05-31 23:32:18]

I'll get the bitness figured out and upload a new version - with the
now-extraneous code from create_installer_archive.py deleted.

https://codereview.chromium.org/2021353003/diff/20001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/2021353003/diff/20001/build/vs_toolchain.py#n...
build/vs_toolchain.py:84: vs_runtime_dll_dirs = [ 'C:/Windows/System32',
'C:/Windows/SysWOW64' ]
I'm not sure. That's worrisome that gyp and gn behave differently.

While investigating I found that the behavior is also different depending on
whether you are running 64-bit python or not. Sigh...

https://codereview.chromium.org/2021353003/diff/20001/build/vs_toolchain.py#n...
build/vs_toolchain.py:208: ucrt_src_dir = os.path.join(source_dir, 'downlevel',
'api-ms-win-*.dll')
On 2016/05/31 21:57:44, scottmg wrote:
> Maybe ucrt_src_pattern or _glob instead.

Done.

[brucedawson, 2016-06-01 00:56:39]

Description was changed from

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when these
directories are created.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

BUG=611934,585556
==========

to

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when these
directories are created.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934,585556, 610336
==========

[brucedawson, 2016-06-01 00:56:39]

brucedawson@chromium.org changed reviewers:
+ gab@chromium.org

[brucedawson, 2016-06-01 01:03:20]

Okay, I think this is good now.

In addition to fixing the mini_installer and isolate builds for non-depot-tools
builds, this also should fix gab's complaint about mini-installer never NOPing
on his machine. And it should fix some problems with deleting 'out' directories,
due to getting rid of a read-only file.

gab - to test this you'll need to delete all instances of msvcrt20.dll and
msvcrt40.dll from your output directories. With this patch applied they should
not come back. Let me know.

[brucedawson, 2016-06-01 01:09:48]

Description was changed from

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when these
directories are created.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934,585556, 610336
==========

to

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when 'gn gen'
or 'gn args' runs.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934,585556, 610336
==========

[scottmg, 2016-06-01 16:54:23]

lgtm

https://codereview.chromium.org/2021353003/diff/40001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/2021353003/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:88: x64_path = 'System32' if bitness == '64bit' else
'Sysnative'
Ah yes, that's simpler since we can cover our ears about 32 bit OSs.

https://codereview.chromium.org/2021353003/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:90: vs_runtime_dll_dirs = [x64_path,
'C:\Windows\SysWOW64']
r'' or escape the \s here.

[brucedawson, 2016-06-01 17:25:33]

https://codereview.chromium.org/2021353003/diff/40001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/2021353003/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:88: x64_path = 'System32' if bitness == '64bit' else
'Sysnative'
On 2016/06/01 16:54:23, scottmg wrote:
> Ah yes, that's simpler since we can cover our ears about 32 bit OSs.

Copied from depot_tools\win_toolchain\package_from_installed.py

https://codereview.chromium.org/2021353003/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:90: vs_runtime_dll_dirs = [x64_path,
'C:\Windows\SysWOW64']
On 2016/06/01 16:54:23, scottmg wrote:
> r'' or escape the \s here.

D'oh! Good catch. Thanks. It would be nice to have a presubmit check for
unescaped strings that contain :\ or C:\

[gab, 2016-06-01 17:44:39]

On 2016/06/01 01:03:20, brucedawson wrote:
> Okay, I think this is good now.
> 
> In addition to fixing the mini_installer and isolate builds for
non-depot-tools
> builds, this also should fix gab's complaint about mini-installer never NOPing
> on his machine. And it should fix some problems with deleting 'out'
directories,
> due to getting rid of a read-only file.
> 
> gab - to test this you'll need to delete all instances of msvcrt20.dll and
> msvcrt40.dll from your output directories. With this patch applied they should
> not come back. Let me know.

Works :-), thanks!

[gab, 2016-06-01 17:45:08]

On 2016/06/01 17:44:39, gab wrote:
> On 2016/06/01 01:03:20, brucedawson wrote:
> > Okay, I think this is good now.
> > 
> > In addition to fixing the mini_installer and isolate builds for
> non-depot-tools
> > builds, this also should fix gab's complaint about mini-installer never
NOPing
> > on his machine. And it should fix some problems with deleting 'out'
> directories,
> > due to getting rid of a read-only file.
> > 
> > gab - to test this you'll need to delete all instances of msvcrt20.dll and
> > msvcrt40.dll from your output directories. With this patch applied they
should
> > not come back. Let me know.
> 
> Works :-), thanks!

And rubberstamp owner lgtm for create_installer_archive.py

[brucedawson, 2016-06-01 17:47:09]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2016-06-01 17:47:10]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2021353003/#ps60001
(title: "Make path a raw string")

[commit-bot: I haz the power, 2016-06-01 17:47:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2021353003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2021353003/60001

[commit-bot: I haz the power, 2016-06-01 18:37:35]

Description was changed from

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when 'gn gen'
or 'gn args' runs.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934,585556, 610336
==========

to

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when 'gn gen'
or 'gn args' runs.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934,585556, 610336
==========

[commit-bot: I haz the power, 2016-06-01 18:37:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-01 19:11:30]

Description was changed from

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when 'gn gen'
or 'gn args' runs.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934,585556, 610336
==========

to

==========
Copy local CRT DLLs to gn output directories

When DEPOT_TOOLS_WIN_TOOLCHAIN=0 it is assumed that the developer has a
local install of Visual Studio, and therefore the CRT DLLs don't need to
be copied to the output directory in order to run binaries. However,
when creating packages for running elsewhere (isolates or the
mini_installer) those DLLs are needed. In order to avoid special cases
elsewhere this change copies them to the output directories when 'gn gen'
or 'gn args' runs.

This change also makes the newly copied files writable so that they can
easily be deleted. For backwards compatibility it also makes the files
writable before deleting them. This avoids a problem with pgort140.dll
which can end up read-only and thus impossible to copy over or delete
without first removing the read-only flag.

This change also removes the need for create_installer_archive.py to
copy CRT DLLs, which should avoid bugs with over-copying of these DLLs.
This change also updates that script to say that ucrtbase*.dll is
required.

BUG=611934,585556, 610336

Committed: https://crrev.com/e7bd0348e692e635c039328cd06aac1ce23b7d4f
Cr-Commit-Position: refs/heads/master@{#397189}
==========

[commit-bot: I haz the power, 2016-06-01 19:11:31]

Patchset 4 (id:??) landed as
https://crrev.com/e7bd0348e692e635c039328cd06aac1ce23b7d4f
Cr-Commit-Position: refs/heads/master@{#397189}