Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Fix configure with spaces in CC #2949

Closed
wants to merge 2 commits into from
Closed

Conversation

Hello71
Copy link

@Hello71 Hello71 commented Mar 16, 2012

For testing, all I can say is "it works on my computer".

@TooTallNate
Copy link

What is the CC variable set to on your computer?

@Hello71
Copy link
Author

Hello71 commented Mar 16, 2012

ccache gcc.

@@ -149,7 +149,7 @@ def pkg_config(pkg):
def host_arch_cc():
"""Host architecture check using the CC command."""

p = subprocess.Popen([CC, '-dM', '-E', '-'],
p = subprocess.Popen(CC.split(' ') + ['-dM', '-E', '-'],

Choose a reason for hiding this comment

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

You probably want CC.split() + ... here (note that no arguments are given to split).

Copy link
Author

Choose a reason for hiding this comment

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

As I've said, I have no clue how Python works. I just read the documentation. Hm. On reading it again, it seems like CC.strip().split() is the closest implementation.

Choose a reason for hiding this comment

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

Sure, CC.strip().split() would be fine. Change it to that and I can merge. Have you signed the CLA?

Copy link
Author

Choose a reason for hiding this comment

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

Upon reading the documentation a third time, it appears that yes, CC.split() is the best option.

Copy link
Author

Choose a reason for hiding this comment

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

See the latest added commit. On a side note, it would be a sensible option to rebase the two commits together before pushing to the main repo. Yes, I've signed the CLA.

@TooTallNate
Copy link

Ahh I see. Well ok, seems fine to me then.

Instead of `CC.split(' ')`.
@TooTallNate
Copy link

Ok thanks @Hello71! Merged in 5abcdc9.

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.

2 participants