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

Fix build failure when clang installed and not used #339

Merged
merged 1 commit into from
Apr 2, 2013

Conversation

gnprice
Copy link
Contributor

@gnprice gnprice commented Apr 2, 2013

This better targets the shorten-64-to-32 workaround, to fix #325.

The previous version of this logic was causing us to pass the flag when

  • clang is on the PATH, and
  • $CC, or 'clang' if CC unset, accepts the flag.

But this is totally wrong if we have clang installed, haven't set $CC, and are going to end up using gcc. We end up passing the flag to gcc, which rejects it.

The real fix is to put this in the autoconf goo in MRI upstream -- the only correct way to decide whether to pass this flag is after we know exactly what compiler we're using and can test if that compiler accepts the flag. But we can do better than before by approximating autoconf's choice of compiler as $CC if set, 'cc' otherwise (which will typically be a symlink to gcc or clang or another.)

The previous version of this logic was causing us to
pass the flag when
 * clang is on the PATH, and
 * $CC, or 'clang' if CC unset, accepts the flag.
But this is totally wrong if we have clang installed,
haven't set $CC, and are going to end up using gcc.
We end up passing the flag to gcc, which rejects it.

The real fix is to put this in the autoconf goo in MRI
upstream -- the only correct way to decide whether to pass
this flag is after we know exactly what compiler we're using
and can test if that compiler accepts the flag.  But we can
do better than before by approximating autoconf's choice of
compiler as $CC if set, 'cc' otherwise (which will typically
be a symlink to gcc or clang or another.)

Fixes: rbenv#319
  and rbenv#325, which is a dupe.
@sferik
Copy link
Contributor

sferik commented Apr 2, 2013

Thanks, Greg.

sferik added a commit that referenced this pull request Apr 2, 2013
Fix build failure when clang installed and not used
@sferik sferik merged commit a585d0b into rbenv:master Apr 2, 2013
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.

gcc does not support "-Wno-error=shorten-64-to-32"
2 participants