New issue
Advanced search Search tips

Issue 863130 link

Starred by 1 user
Status: Verified
Owner: ----
Closed: Jul 30
Cc:
brucedaw...@chromium.org
h...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Windows: clang/scripts/update.py fails with DEPOT_TOOLS_WIN_TOOLCHAIN=0 and non-standard VS install location

Project Member Reported by marshall@chromium.org, Jul 12 
Chrome Version: 69.0.3483.0 (master revision 68bf9707c1 (#572879))
OS: Windows 10

What steps will reproduce the problem?
(1) Run `gclient runhooks` with `DEPOT_TOOLS_WIN_TOOLCHAIN=0 GYP_MSVS_OVERRIDE_PATH=<path>` environment variables.

What is the expected result?
Runhooks should succeed.

What happens instead?
clang/scripts/update.py fails with the following error:

________ running 'Y:\work\CEF3_git\depot_tools\win_tools-2_7_6_bin\python\bin\python.exe src/build/vs_toolchain.py update --force' in 'Y:\work\CEF3_git\chromium'
________ running 'Y:\work\CEF3_git\depot_tools\win_tools-2_7_6_bin\python\bin\python.exe src/tools/clang/scripts/update.py' in 'Y:\work\CEF3_git\chromium'
Downloading https://commondatastorage.googleapis.com/chromium-browser-clang/Win/clang-335864-1.tgz .......... Done.
Traceback (most recent call last):
  File "src/tools/clang/scripts/update.py", line 936, in <module>
    sys.exit(main())
  File "src/tools/clang/scripts/update.py", line 932, in main
    return UpdateClang(args)
  File "src/tools/clang/scripts/update.py", line 479, in UpdateClang
    CopyDiaDllTo(os.path.join(LLVM_BUILD_DIR, 'bin'))
  File "src/tools/clang/scripts/update.py", line 406, in CopyDiaDllTo
    GetWinSDKDir()
  File "src/tools/clang/scripts/update.py", line 395, in GetWinSDKDir
    vs_path = vs_toolchain.DetectVisualStudioPath()
  File "Y:\work\CEF3_git\chromium\src\build\vs_toolchain.py", line 154, in DetectVisualStudioPath
    ' not found.') % (version_as_year))
Exception: Visual Studio Version 2017 (from GYP_MSVS_VERSION) not found.

Please use labels and text to provide additional information.
This was likely broken in https://crrev.com/75fb92d6ee87b8dad7edeb8f2bcbba9b2597055d.

If `GYP_MSVS_OVERRIDE_PATH` is set that value should be used instead of calling `vs_toolchain.DetectVisualStudioPath()`. See for example _LoadToolchainEnv [1].

[1] https://cs.chromium.org/chromium/src/build/toolchain/win/setup_toolchain.py?type=cs&q=_LoadToolchainEnv&g=0&l=86

Comment 1 by marshall@chromium.org, Jul 12

Description: Show this description

Comment 2 by marshall@chromium.org, Jul 12 

This patch fixes the problem for me:

diff --git tools/clang/scripts/update.py tools/clang/scripts/update.py
index 226c3981a3bb..560a58070883 100755
--- tools/clang/scripts/update.py
+++ tools/clang/scripts/update.py
@@ -392,7 +392,10 @@ def GetWinSDKDir():
   if bool(int(os.environ.get('DEPOT_TOOLS_WIN_TOOLCHAIN', '1'))):
     dia_path = os.path.join(win_sdk_dir, '..', 'DIA SDK', 'bin', 'amd64')
   else:
-    vs_path = vs_toolchain.DetectVisualStudioPath()
+    if 'GYP_MSVS_OVERRIDE_PATH' not in os.environ:
+      vs_path = vs_toolchain.DetectVisualStudioPath()
+    else:
+      vs_path = os.environ['GYP_MSVS_OVERRIDE_PATH']
     dia_path = os.path.join(vs_path, 'DIA SDK', 'bin', 'amd64')
 
   dia_dll = os.path.join(dia_path, DIA_DLL[msvs_version])

Comment 3 by brucedaw...@chromium.org, Jul 12 

Can you submit a patch? Your fix looks reasonable.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 30 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d6dca0cbac88e59635b7ab8128fd79a143e2631

commit 1d6dca0cbac88e59635b7ab8128fd79a143e2631
Author: Marshall Greenblatt <marshall@chromium.org>
Date: Mon Jul 30 16:04:50 2018

Windows: Fix clang/scripts/update.py failure

Fix clang/scripts/update.py failure with DEPOT_TOOLS_WIN_TOOLCHAIN=0
and non-standard VS install location.

If GYP_MSVS_OVERRIDE_PATH is set that value should be used instead of
calling vs_toolchain.DetectVisualStudioPath().

R=brucedawson@chromium.org

Bug:  863130 
Change-Id: Id72b1a22f74c5fa9174a76d611fef9ca69bb8319
Reviewed-on: https://chromium-review.googlesource.com/1135761
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Marshall Greenblatt <marshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579056}
[modify] https://crrev.com/1d6dca0cbac88e59635b7ab8128fd79a143e2631/tools/clang/scripts/update.py

Comment 6 by marshall@chromium.org, Jul 30

Status: Verified (was: Untriaged)
►

Sign in to add a comment