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.

Sign up for GitHub

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

Detect whether VC++ compiler can be found in configure.py #2086

Merged
merged 5 commits into from
Oct 23, 2022
Merged

Detect whether VC++ compiler can be found in configure.py #2086

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87b8965
Improve error messages when VS cannot be found
randomascii Jul 26, 2021
34d03bd
Merge branch 'ninja-build:master' into compiler_detect
randomascii Feb 17, 2022
3ef318c
Merge branch 'ninja-build:master' into compiler_detect
randomascii Feb 17, 2022
b5fa2d5
Fix function and parameter names, and comment
randomascii Mar 9, 2022
69fb2ce
Merge branch 'master' into compiler_detect
jhasse Aug 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions configure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,16 @@ def binary(name):
else:
n.variable('ar', configure_env.get('AR', 'ar'))

def SearchPath(exe_name):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rename this to search_windows_path() since this doesn't handle anything else and uses the same naming convention as the rest of this file.

"""Find an executable (.exe, .bat, whatever) in the system path."""
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The comment and naming used here suggest that this behaves like the Win32 SearchPath() function and will probe for <exe_name>.bat, <exe_name>,exe or <exe_name>.cmd, which this function does not. Using "find a file in the current Windows path" would be more accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point about the name and the comment. The function is OS-generic so I renamed it to search_system_path - it could be used on any OS, it just currently isn't. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the name change, I would just mention that the function is not OS-generic though. I.e. on Unix you would want to use ':', not ';' for the split, and you would need to handle empty items, which translate to "search the current path" (and frequent source of conflict and security issues, happens all the time with LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/some/new/path, when LD_LIBRARY_PATH was empty, and now your library search path will always start in the current directory, boom).

Please do not handle this here, but that's why search_windows_path() was a simpler name :-) And path searching is always trickier than expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points. I could rename to search_windows_path, or leave it as-is. Fixing the path separator is easy enough (os.pathsep instead of ';', perhaps a worthwhile change anyway) but it sounds like it still wouldn't be truly generic.

for dir in os.environ['path'].split(';'):
path = os.path.join(dir, exe_name)
if os.path.exists(path):
return path

if platform.is_msvc():
if not SearchPath('cl.exe'):
raise Exception('cl.exe not found. Run again from the Developer Command Prompt for VS')
cflags = ['/showIncludes',
'/nologo', # Don't print startup banner.
'/Zi', # Create pdb with debug info.
Expand Down