Use visual studio toolchain from depot_tools for PDFium compilation

Use visual studio toolchain from depot_tools for PDFium compilation

Change to use visual studio toolchain from depot_tools by default.
Setting DEPOT_TOOLS_WIN_TOOLCHAIN=0 allows compilation using system
toolchain as before.

Using toolchain from depot_tools unifies the compilation
environment, and brings the benefits of automated update, bug fixes
etc.

Committed: https://pdfium.googlesource.com/pdfium/+/590f2d9e057a0d5b17a9706affd3c6115265021b

[Wei Li, 2016-04-15 20:13:46]

weili@chromium.org changed reviewers:
+ brucedawson@chromium.org, tsepez@chromium.org

[Wei Li, 2016-04-15 20:13:47]

PTAL, thanks

[Tom Sepez, 2016-04-15 20:20:37]

tsepez@chromium.org changed reviewers:
+ thestig@chromium.org

[Tom Sepez, 2016-04-15 20:20:38]

Deferring to bruce (for windows-fu) and lei (for python-fu).

[Wei Li, 2016-04-15 20:49:00]

FYI, find_depot_tools and vs_toolchain are copies from chromium with only a path
setting change in vs_toolchain. Not sure how to show the diff though.

[brucedawson, 2016-04-15 21:38:07]

On 2016/04/15 20:49:00, Wei Li wrote:
> FYI, find_depot_tools and vs_toolchain are copies from chromium with only a
path
> setting change in vs_toolchain. Not sure how to show the diff though.

The change looks reasonable. I would be sure to test it on Windows with gn and
gyp builds - luckily testing with VS 2013 is no longer necessary.

If you need me to test it on Windows I can.

[Lei Zhang, 2016-04-15 21:58:51]

Are the Python files just exact copies of the same files in the Chromium repo?

[Wei Li, 2016-04-16 01:05:01]

@brucedawson, thanks, I ran through GYP/GN chromium builds without problems.

@Lei, other than the copy right change, find_depot_tools.py is an exact copy,
vs_tooldchain.py has some path change on line 17-21.

thanks for the review!

[Lei Zhang, 2016-04-18 20:58:03]

https://codereview.chromium.org/1897523002/diff/1/build/find_depot_tools.py
File build/find_depot_tools.py (right):

https://codereview.chromium.org/1897523002/diff/1/build/find_depot_tools.py#n...
build/find_depot_tools.py:2: # Copyright 2016 PDFium Authors. All rights
reserved.
IANAL, but we've copied other files from Chromium and just kept the copyright
notice as is. So let's be consistent and not have multiple copyright lines.

https://codereview.chromium.org/1897523002/diff/1/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/1897523002/diff/1/build/vs_toolchain.py#newco...
build/vs_toolchain.py:21: sys.path.insert(0, os.path.join(src_root, 'build',
'gyp', 'pylib'))
On my machine, sys.path starts out as:

['', '/foo', '/bar', ...]

After these two inserts, I'd get:

['/path/to/build/gyp/pylib', '', '/path/to/build/gyp/pylib', '/foo', '/bar',
...]

Is this intentional?

[Wei Li, 2016-04-18 22:22:01]

thanks

https://codereview.chromium.org/1897523002/diff/1/build/find_depot_tools.py
File build/find_depot_tools.py (right):

https://codereview.chromium.org/1897523002/diff/1/build/find_depot_tools.py#n...
build/find_depot_tools.py:2: # Copyright 2016 PDFium Authors. All rights
reserved.
On 2016/04/18 20:58:02, Lei Zhang wrote:
> IANAL, but we've copied other files from Chromium and just kept the copyright
> notice as is. So let's be consistent and not have multiple copyright lines.

Done.

https://codereview.chromium.org/1897523002/diff/1/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/1897523002/diff/1/build/vs_toolchain.py#newco...
build/vs_toolchain.py:21: sys.path.insert(0, os.path.join(src_root, 'build',
'gyp', 'pylib'))
On 2016/04/18 20:58:03, Lei Zhang wrote:
> On my machine, sys.path starts out as:
> 
> ['', '/foo', '/bar', ...]
> 
> After these two inserts, I'd get:
> 
> ['/path/to/build/gyp/pylib', '', '/path/to/build/gyp/pylib', '/foo', '/bar',
> ...]
> 
> Is this intentional?

yes, paths here are absolute paths same as build/gyp_pdfium:18. It should also
be ok to use relative ones.

[brucedawson, 2016-04-22 23:43:27]

I tried testing this on my stand-alone pdfium repo and hit lots of errors. Some
make no sense (find_depot_tools.py is newly added - of course it's not in the
index). The patch looks fine so I guess there's something wrong with my repo?
Sorry - no can test. 

C:\src\pdfium\pdfium>git cl patch 1897523002
error: patch failed: .gitignore:1
Falling back to three-way merge...
Applied patch to '.gitignore' with conflicts.
error: patch failed: DEPS:144
Falling back to three-way merge...
Applied patch to 'DEPS' cleanly.
Falling back to three-way merge...
error: build/find_depot_tools.py: does not exist in index
error: cannot read the current contents of 'build/find_depot_tools.py'
error: build/find_depot_tools.py: patch does not apply
error: build/gyp_pdfium: does not exist in index
Falling back to three-way merge...
error: build/vs_toolchain.py: does not exist in index
error: cannot read the current contents of 'build/vs_toolchain.py'
error: build/vs_toolchain.py: patch does not apply
Failed to apply the patch

[Wei Li, 2016-04-23 00:28:43]

On 2016/04/22 23:43:27, brucedawson wrote:
> I tried testing this on my stand-alone pdfium repo and hit lots of errors.
Some
> make no sense (find_depot_tools.py is newly added - of course it's not in the
> index). The patch looks fine so I guess there's something wrong with my repo?
> Sorry - no can test. 
> 
> C:\src\pdfium\pdfium>git cl patch 1897523002
> error: patch failed: .gitignore:1
> Falling back to three-way merge...
> Applied patch to '.gitignore' with conflicts.
> error: patch failed: DEPS:144
> Falling back to three-way merge...
> Applied patch to 'DEPS' cleanly.
> Falling back to three-way merge...
> error: build/find_depot_tools.py: does not exist in index
> error: cannot read the current contents of 'build/find_depot_tools.py'
> error: build/find_depot_tools.py: patch does not apply
> error: build/gyp_pdfium: does not exist in index
> Falling back to three-way merge...
> error: build/vs_toolchain.py: does not exist in index
> error: cannot read the current contents of 'build/vs_toolchain.py'
> error: build/vs_toolchain.py: patch does not apply
> Failed to apply the patch

It is changing to GN world now :), Let me rebase.

[Wei Li, 2016-04-23 03:07:48]

Rebased, pls try, thanks!

[brucedawson, 2016-04-27 00:57:34]

On 2016/04/23 03:07:48, Wei Li wrote:
> Rebased, pls try, thanks!

Yep, seems to work. I just did a debug build of pdfium using the depot_tools VS
2015 toolchain, and part of a release build. I then did a gn debug component
build. No problems with any of them.

So lgtm.

[Lei Zhang, 2016-04-27 20:13:35]

lgtm

[Wei Li, 2016-04-27 20:42:44]

The CQ bit was checked by weili@chromium.org

[commit-bot: I haz the power, 2016-04-27 20:42:50]

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

[commit-bot: I haz the power, 2016-04-27 21:06:59]

Description was changed from

==========
Use visual studio toolchain from depot_tools for PDFium compilation

Change to use visual studio toolchain from depot_tools by default.
Setting DEPOT_TOOLS_WIN_TOOLCHAIN=0 allows compilation using system
toolchain as before.

Using toolchain from depot_tools unifies the compilation
environment, and brings the benefits of automated update, bug fixes
etc.
==========

to

==========
Use visual studio toolchain from depot_tools for PDFium compilation

Change to use visual studio toolchain from depot_tools by default.
Setting DEPOT_TOOLS_WIN_TOOLCHAIN=0 allows compilation using system
toolchain as before.

Using toolchain from depot_tools unifies the compilation
environment, and brings the benefits of automated update, bug fixes
etc.

Committed:
https://pdfium.googlesource.com/pdfium/+/590f2d9e057a0d5b17a9706affd3c6115265...
==========

[commit-bot: I haz the power, 2016-04-27 21:07:00]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/590f2d9e057a0d5b17a9706affd3c6115265...

[Wei Li, 2016-04-27 23:14:22]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1922373003/ by weili@chromium.org.

The reason for reverting is: Clang build is broken. drm build has some problem.
Will revert this change for now..