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

Issue 2653473002: Allow building Chrome with local VC++ 2017 install (Closed)

Created:
3 years, 11 months ago by brucedawson
Modified:
3 years, 7 months ago
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Allow building Chrome with local VC++ 2017 install This updates the toolchain scripts enough so that it is possible to build Chrome with VC++ 2017 if DEPOT_TOOLS_WIN_TOOLCHAIN=0 and GYP_MSVS_VERSION=2017, followed by gclient runhooks. There are still some local patches needed to make it work, and treat_warnings_as_errors = false and is_component_build = false in args.gn may be needed initially, but my local builds are working. The toolchain code should be updated eventually to use COM to locate the VC++ install location, instead of the hard-coded path which is being used as a temporary measure in this change. R=scottmg@chromium.org BUG=683729 Review-Url: https://codereview.chromium.org/2653473002 Cr-Commit-Position: refs/heads/master@{#445329} Committed: https://chromium.googlesource.com/chromium/src/+/adddab40528d1fb6dd94c0e4d2bc794ffccf194a

Patch Set 1 #

Total comments: 6

Patch Set 2 : Rename _CopyRuntime2015 to _CopyUCRTRuntime #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -14 lines) Patch
M build/toolchain/win/setup_toolchain.py View 1 chunk +7 lines, -2 lines 0 comments Download
M build/vs_toolchain.py View 1 4 chunks +26 lines, -12 lines 2 comments Download

Messages

Total messages: 13 (5 generated)
brucedawson
PTAL
3 years, 11 months ago (2017-01-22 21:29:26 UTC) #1
scottmg
lgtm https://codereview.chromium.org/2653473002/diff/1/build/vs_toolchain.py File build/vs_toolchain.py (right): https://codereview.chromium.org/2653473002/diff/1/build/vs_toolchain.py#newcode146 build/vs_toolchain.py:146: # The VC++ 2017 install location needs to ...
3 years, 11 months ago (2017-01-22 23:43:35 UTC) #2
brucedawson
https://codereview.chromium.org/2653473002/diff/1/build/vs_toolchain.py File build/vs_toolchain.py (right): https://codereview.chromium.org/2653473002/diff/1/build/vs_toolchain.py#newcode146 build/vs_toolchain.py:146: # The VC++ 2017 install location needs to be ...
3 years, 11 months ago (2017-01-23 06:46:03 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2653473002/20001
3 years, 11 months ago (2017-01-23 06:46:31 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/adddab40528d1fb6dd94c0e4d2bc794ffccf194a
3 years, 11 months ago (2017-01-23 06:58:41 UTC) #9
Sébastien Marchand
https://codereview.chromium.org/2653473002/diff/20001/build/vs_toolchain.py File build/vs_toolchain.py (right): https://codereview.chromium.org/2653473002/diff/20001/build/vs_toolchain.py#newcode177 build/vs_toolchain.py:177: return '150' Is it true? The files in 4e8a360587a3c8ff3fa46aa9271e982bf3e948ec ...
3 years, 7 months ago (2017-05-10 21:23:49 UTC) #11
brucedawson
https://codereview.chromium.org/2653473002/diff/20001/build/vs_toolchain.py File build/vs_toolchain.py (right): https://codereview.chromium.org/2653473002/diff/20001/build/vs_toolchain.py#newcode177 build/vs_toolchain.py:177: return '150' On 2017/05/10 21:23:49, Sébastien Marchand wrote: > ...
3 years, 7 months ago (2017-05-10 21:32:32 UTC) #12
Sébastien Marchand
3 years, 7 months ago (2017-05-11 14:46:58 UTC) #13
Message was sent while issue was closed.
I was looking at pgort140.dll, but it looks like all the DLLs
in 4e8a360587a3c8ff3fa46aa9271e982bf3e948ec use ends with 140.dll (i.e.
searching for *150.dll doesn't return anything).

The problem is probably that this "_VersionNumber" function is only used to
retrieve the PGO runtime library (pgort140.dll), the other runtime
dependencies are don't use it, see
https://cs.chromium.org/chromium/src/build/vs_toolchain.py?l=236&rcl=37704861...

I'll try to clean this up.

On Wed, May 10, 2017 at 5:32 PM, <brucedawson@chromium.org> wrote:

>
> https://codereview.chromium.org/2653473002/diff/20001/
> build/vs_toolchain.py
> File build/vs_toolchain.py (right):
>
> https://codereview.chromium.org/2653473002/diff/20001/
> build/vs_toolchain.py#newcode177
> build/vs_toolchain.py:177: return '150'
> On 2017/05/10 21:23:49, Sébastien Marchand wrote:
> > Is it true? The files in 4e8a360587a3c8ff3fa46aa9271e982bf3e948ec
> seems to be
> > using the 140 version number, any idea why?
>
> I think VS 2017 uses 140, 141, and 150, depending on context. What
> specific files are you referring to?
>
> https://codereview.chromium.org/2653473002/
>



-- 
Sébastien Marchand | Software Developer | sebmarchand@google.com

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698