Improve DDR support to allow use of VS 2017 #2798
Conversation
|
The changes mainly stet up the path to cl.exe in VS2017 Reviewer: @keithc-ca , @pshipton |
|
@genie-omr build all |
ChengJin01
force-pushed
the
OMR_Fix_issues_JDK11_Windows
branch
from
a96a222 to
7e1e58f
Compare
|
@keithc-ca what are your thoughts on this change? |
| ] | ||
| diasdk_dir1="$vc_bin_dir/../../DIA SDK" | ||
| diasdk_dir2="$vc_bin_dir/../../../DIA SDK" | ||
| diasdk_dir3="$vc_bin_dir/../../../../../../../DIA SDK" |
There was a problem hiding this comment.
For diasdk_dir3, please provide example absolute paths that are covered by this addition.
There was a problem hiding this comment.
diasdk_dir3 is used to cover the case of VS2017 (works for both Professional and Community version). The reason is that the path to cl.exe has changed on VS2017.
Administrator@JCHTORWIN101 /cygdrive/c/Program Files (x86)/Microsoft Visual Studio/2017/Professional
$ ls
Common7 'DIA SDK' JS SDK VB VSSDK
CoreCon ImportProjects Licenses 'Team Tools' VC Web
DesignTools JavaScript MSBuild TS VC# Xml
Administrator@JCHTORWIN101 /cygdrive/c/Program Files (x86)/Microsoft Visual Studio/2017/Professional
$ find . -name cl.exe
./VC/Tools/MSVC/14.14.26428/bin/Hostx64/x64/cl.exe
./VC/Tools/MSVC/14.14.26428/bin/Hostx64/x86/cl.exe
./VC/Tools/MSVC/14.14.26428/bin/Hostx86/x64/cl.exe
./VC/Tools/MSVC/14.14.26428/bin/Hostx86/x86/cl.exe
e.g.
In VS2017 Professional, the path to DIA SDK: C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\DIA SDK while vc_bin_dir is C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\bin\Hostx86\x64\cl.exe. So, we need to step backwards from the path to cl.exe by 7 levels to reach the DIA SDK directory.
There was a problem hiding this comment.
Thanks for the explanation.
|
The change to PdbScanner.cpp looks good, although I would have preferred the case labels in alphabetical order. |
|
@genie-omr build all |
|
I think the title of the commit and this PR should make it clearer that this is about improving DDR support to allow use of VS 2017. |
ChengJin01
force-pushed
the
OMR_Fix_issues_JDK11_Windows
branch
from
7e1e58f to
3a37dfe
Compare
|
Already updated the title and the description of commit/PR. |
The change is to improve DDR support in script/code so as to allow use of VS 2017 on Windows. Signed-off-by: CHENGJin <[email protected]>
ChengJin01
force-pushed
the
OMR_Fix_issues_JDK11_Windows
branch
from
3a37dfe to
dfb5021
Compare
|
@genie-omr build all |
|
@ChengJin01 Can this PR title be changed to match the commit message: 'Improve DDR support to allow use of VS 2017', please? |
ChengJin01
changed the title
|
Already changed the PR's title. |
The change is to improve DDR support in script/code
so as to allow use of VS 2017 on Windows.
Signed-off-by: CHENGJin [email protected]