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

[python] correctly handle full path to compiler in CC and CXX env vars #2345

Merged
merged 1 commit into from Aug 28, 2019

Conversation

StrikerRUS
Copy link
Collaborator

For instance, CC can be /usr/bin/gcc. Refer to #2339.

@jameslamb Seems that the regex from #2339 for R-package will produce false positive result for something like C:/Users/gccharlee/pgcc-8 (any intermediate folder name which starts with gcc).

@jameslamb
Copy link
Collaborator

Ah! Forgot a $. Will make a PR momentarily, thanks for catching it.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Looks good to me! This does mean R and Python will have slightly different behavior (since you are using startswith() but I think it covers most cases and is definitely an improvement over the previous state.

@StrikerRUS
Copy link
Collaborator Author

@jameslamb Thanks for your review!

This does mean R and Python will have slightly different behavior (since you are using startswith() but I think it covers most cases and is definitely an improvement over the previous state.

Yeah, I decided to leave startswith because of its' better readability and simplicity in comparison with regex. And the most important - it doesn't require additional imports.

BTW, it's not critical since we run this compiler check only for macOS, but the regex from #2339 doesn't work for Windows (e.g. gcc.exe).

@StrikerRUS StrikerRUS merged commit 5f5dcff into master Aug 28, 2019
@StrikerRUS StrikerRUS deleted the python_setup_macos branch August 28, 2019 11:43
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants