Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Visual Studio detection scripts #1008

Closed
tobil4sk opened this issue Oct 3, 2022 · 8 comments · Fixed by #1035
Closed

Update Visual Studio detection scripts #1008

tobil4sk opened this issue Oct 3, 2022 · 8 comments · Fixed by #1035

Comments

@tobil4sk
Copy link
Member

tobil4sk commented Oct 3, 2022

It looks like the scripts for detecting MSVC installations are out of date, because they have specific checks for VS2017 but not for VS2019 or VS2022. We should update them so they work for newer versions.

See:
https://github.com/HaxeFoundation/hxcpp/blob/master/toolchain/msvc-setup.bat
https://github.com/HaxeFoundation/hxcpp/blob/master/toolchain/msvc64-setup.bat

@justin-espedal
Copy link
Contributor

Quick recap:

  1. VS2017 detection was originally added in.
  2. It was shortly replaced with a vswhere-based lookup.
  3. After a few months, an explicit VS2017 lookup was added back in as a fallback, though I don't understand why exactly. In what situation does vswhere fail to work?

In general, VS2017 and all newer versions should be detected with the vswhere tool, since all of these install vswhere as part of their setup process. It's not clear to me why VS2017 is explicitly included in the script this way. I have only VS2019 installed, and it works fine for me.

I'd be interested to hear why a fallback was desired at all for VS2017.

If somebody who can merge PRs looks into this, I'd appreciate this being looked at as well: #796.

@hughsando
Copy link
Member

I can't recall exactly, but vswhere may have come as a separate download or service pack at some stage, so there may have been a transition. As I understand it, the latest stuff seems to just work. Are you finding a need for additional fallbacks?

@tobil4sk
Copy link
Member Author

This came up after troubleshooting with a user from the Haxe discord who wasn't able to get the compilation to work. I believe they had installed vs2019 via winget:

winget install --id=Microsoft.VisualStudio.2019.BuildTools  -e

However, HXCPP was giving errors about how it it wasn't able to locate vs2017.

@hughsando
Copy link
Member

I don't have a problem with fallbacks - especially with a well defined used case - so if they fix things then good.
They seem slightly brittle to me (specific install directory etc) so a generic solution would be better if we had one.
Do you know if it is possible to install vswhere using winget too? That might be a neat solution.
Another solution might setting NO_AUTO_MSVC, which will just try "cl.exe" if it can be found in the path.
We might also consider a new haxelib "hxcpp_vswhere" that includes the vswhere.exe so we can always find it.

@tobil4sk
Copy link
Member Author

Do you know if it is possible to install vswhere using winget too?

Looks like the answer to that is no unfortunately, for now at least: microsoft/vswhere#222.

Here is the wiki page with the available installation methods:
https://github.com/microsoft/vswhere/wiki/Installing

Perhaps a solution would be to add proper instructions on how to install a minimum working msvc version somewhere in the manual. If it is clear to users that they need to have vswhere so MSVC can be located, then it's less of an issue.

@tobil4sk
Copy link
Member Author

Although, it looks like vswhere doesn't find build tools by default, unless an extra flag is added:
microsoft/vswhere#8

@tobil4sk
Copy link
Member Author

tobil4sk commented Oct 16, 2022

-products * is in theory what we need to pass so that build tools are searched as well... but we already have that... Perhaps the -required flag is causing the build tools to be excluded?
https://github.com/HaxeFoundation/hxcpp/blob/master/toolchain/msvc64-setup.bat#L12

nvm, acording to #1010, it works fine as long as vswhere is installed.

@tobil4sk
Copy link
Member Author

Ok, so apparently the winget package does already exist:

winget install -e --id Microsoft.VisualStudio.Locator

See: https://winget.run/pkg/Microsoft/VisualStudio.Locator

I assumed it wasn't because it wasn't mentioned in the documentation.

In that case, we might just want to update the docs to mention that vswhere is required, and also update the error message here:
https://github.com/HaxeFoundation/hxcpp/blob/master/toolchain/msvc64-setup.bat#L15-L21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants