kdevelop-msvc.bat finds VS-2017 based on a registry key on Windows.
ClosedPublic

Authored by Petross404 on Jan 1 2019, 11:01 PM.

Details

Summary

Some months earlier, we discussed here about kdevelop-msvc.bat finding dynamically a VS2017 installation based on registry entries.

Although it works on my Dell laptop (Windows 7 Ultimate x64), I haven't tested it anywhere else as I don't have access to another windows machine. So... any takers?

Test Plan

In case some user has installed VS in a non-standard location (i.e. not in C:\Program Files), the script will be able to find the VS tools by not making the assumption that they are installed on C:\.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Petross404 requested review of this revision.Jan 1 2019, 11:01 PM
Petross404 created this revision.
mwolff requested changes to this revision.Jan 27 2019, 7:23 PM
mwolff added a subscriber: mwolff.

sorry for the long delay, it sounds like a good idea but the diff is super hard to review since it contains lots of unrelated whitespace changes

app/windows/kdevelop-msvc.bat
8

could you please undo the whitespace changes and re-upload the minimal diff?

This revision now requires changes to proceed.Jan 27 2019, 7:23 PM
Petross404 updated this revision to Diff 50446.Jan 28 2019, 7:34 PM

Upload the minimum diff.

PS. Sorry if it's a spam, but I could make a PR in https://invent.kde.org. Which method should I follow for now on?

mwolff accepted this revision.Jan 29 2019, 9:44 AM

you can pick whatever you prefer for the reviews for now.

do you have commit rights? if so, please feel free to push this directly, otherwise someone from us can take care of that for you

app/windows/kdevelop-msvc.bat
8

is vs15 visual studio 2017? I'm always confused by the versioning scheme of visual studio :D

This revision is now accepted and ready to land.Jan 29 2019, 9:44 AM
Petross404 marked an inline comment as done.Jan 29 2019, 10:21 AM

you can pick whatever you prefer for the reviews for now.

do you have commit rights? if so, please feel free to push this directly, otherwise someone from us can take care of that for you

No, AFAIK I dont't have any commit rights. Both at invent.kde and phabricator. So please, take care of this :)

Petross404 added inline comments.Jan 29 2019, 10:34 AM
app/windows/kdevelop-msvc.bat
8

Yeah, I know. VS 2017 = VS15 and VS 2015 = VS14.

This revision was automatically updated to reflect the committed changes.
sredman added a subscriber: sredman.Jul 8 2019, 3:42 PM

Hello

Unfortunately, I have a problem with using the batch file from this diff on my system.

This is without my changes (D21589), exactly the version currently on master

I do not have VS 2017 installed, only VS 2019 Community and VS 2015 Enterprise. As you can see, the path detected is to my VS 2015 installation, but the terminal output is the code path for the VS2017 handling (See lines 36-49 of app/windows/kdevelop-msvc.bat)

Loosely, this seems to be because my Windows registry for HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\VisualStudio\SxS\VS7 says C:\Program Files (x86)\Microsoft Visual Studio 14.0\

@Petross404 are you still around? Do you have any documentation references for how Microsoft expects the registry to look with regard to which key should contain the Visual Studio installation path? (I don't know if such documentation even exists...)

Actually, a pleasant surprise for today is that this whole mess of registry keys and environment variables is mostly unnecessary as of VS 2017, since Microsoft provides a tool which does all the locating work for us: https://github.com/microsoft/vswhere

@Petross404 are you still around? Do you have any documentation references for how Microsoft expects the registry to look with regard to which key should contain the Visual Studio installation path? (I don't know if such documentation even exists...)

No, I don't have any documentation for this. If you are right this could be much more worse since it would mean that the patch only worked for me. I will look into possible bug reports later.