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

Improve make.bat's resiliency to Visual Studio install locations #660

Merged
merged 1 commit into from
Feb 4, 2016
Merged

Improve make.bat's resiliency to Visual Studio install locations #660

merged 1 commit into from
Feb 4, 2016

Conversation

mpderbec
Copy link

@mpderbec mpderbec commented Aug 2, 2015

This is a small change that allows make.bat to respond to non-standard Visual Studio install locations.

The VS90COMNTOOLS variable is always defined globally if Visual Studio 9.0 is installed.

@mrjefftang
Copy link
Collaborator

This should be dependent on the version of Python.

Visual studio 2008 for Python 2.6, 2.7, 3.2
Visual studio 2010 for Python 3.1 - 3.4
Visual studio 2015 for Python 3.5+

@mpderbec
Copy link
Author

mpderbec commented Aug 3, 2015

I'm new to this codebase, so I might not understand exactly how it works. My changes are meant to preserve the status quo -- as you can see in the old version of the batch file, it is always using VS2008 (9.0). So the only thing this PR does is accommodate users that may have installed VS2008 to somewhere other than C:\Program Files (x86)\Microsoft Visual Studio 9.0.

I wouldn't feel comfortable making the changes you propose -- at least not in this particular PR -- because it would significantly change current behavior and I would fear breaking something in the process.

Again-- I may not understand the situation well enough so please let me know if I'm off base.

giampaolo added a commit that referenced this pull request Feb 4, 2016
Improve make.bat's resiliency to Visual Studio install locations
@giampaolo giampaolo merged commit bb13d64 into giampaolo:master Feb 4, 2016
@giampaolo
Copy link
Owner

I tested this works as expected. Thanks (I also added you to CREDITS).

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

Successfully merging this pull request may close these issues.

3 participants