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

Enable autopxd to use Microsoft Visual C++ for preprocessing on Windows #40

Merged
merged 3 commits into from
Aug 3, 2022

Conversation

zooba
Copy link
Contributor

@zooba zooba commented Jul 16, 2022

No description provided.

@zooba
Copy link
Contributor Author

zooba commented Jul 16, 2022

I'm publishing this as a draft, because I may not get time to finish it up, but wanted to share the work in case someone else can pick it up.

Basically, I wanted this to work without installing MinGW or some other build of GCC, so I added handling for when then cpp command is missing on Windows and made it use MSVC instead.

In case you've never seen it before, the VS discovery is how it's supposed to work. There are no suitable registry entries for VS anymore, and the vswhere tool is always in the same location. It does, however, need better handling for when the tool is missing. If the tool is missing, there are no VS installs, which would be an error here anyway! But the error message could be better - similarly if the returned JSON is an empty list (i.e. no VS install included cl.exe).

Also tests, of course, though it appears you don't have Windows CI set up anyway. It seems to work okay anyway though 👍

@zooba zooba marked this pull request as ready for review July 17, 2022 10:50
@zooba
Copy link
Contributor Author

zooba commented Jul 17, 2022

I just rebased and squashed everything as there's nothing interesting in my history.

I wish the Windows tests could be quicker - they are embarrassingly slow. It's just the nature of launching processes on Windows. That's the big heavy process, and because it keeps shelling out to MSVC or GCC multiple times, it's inherently slow. If you'd rather not have the tests run on every PR, I understand

Copy link
Owner

@elijahr elijahr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for contributing. I don't have Windows to test these changes with and am unfamiliar with the stack, so just have a few clarifying questions.

@elijahr
Copy link
Owner

elijahr commented Aug 3, 2022

This is great. Really appreciate it! Will publish a new release with these changes soon.

@elijahr elijahr merged commit b438aa7 into elijahr:master Aug 3, 2022
@elijahr
Copy link
Owner

elijahr commented Aug 3, 2022

This has been released in v2.2.2.

@zooba zooba deleted the msvc branch August 3, 2022 19:23
@jakirkham
Copy link

Thanks Steve! 🙏

Nice to see Windows support here 😄

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 this pull request may close these issues.

3 participants