brew install ruby failure

[tlberglund]

Similar to #13046, brew install ruby is failing for me with a compilation error in readline.c related to the username_completion_function function. I have whipped brew doctor into the best shape I know how, but you'll note it's still complaining of the Xcode command line tools.

Gists of all of the things:
brew install -v ruby (stdout)
brew install -v ruby (stderr)
brew doctor
brew --config

Let me know what other information I can provide, and thanks in advance for your attention.

[slarew]

I just ran brew install --with-doc --with-cscope bash-completion macvim mercurial nginx ruby homebrew/dupes/vim tree on a "fresh" system and it worked with the latest homebrew. I had to temporarily unlink ruby so that homebrew/dupes/vim would build. Otherwise, my problems from #13046 with readline still seem to be fixed. Best of luck to you.

Edit: I know this wasn't exactly an isolated test as I was installing other software as well. Just thought I would report my results.

[2bits]

We could tag this xcode-only and ask sj to look at it.

[tlberglund]

Still no luck here. I got a failure of different kind, but similar to the last. This is a missing header file (and it was the macvim build that failed), whereas the last might be an incorrect header file. All of which suggests a problem with my C compiler, which brew doctor also hints at. I don't need Xcode CLT for this to work, do I?

[tlberglund]

@2bits Speaking as an outsider, it does seem Xcode-related.

[mistydemeo]

ping @samueljohn

[jeffstyr]

Based on the referenced gist, I'm pretty sure the problem is the -L to the SDK in CFLAGS. What's happening is that at configure-time it's finding the headers in /usr/local/Cellar/readline/6.2.2/include, but at link time the CFLAGS are causing the library in /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.7.sdk/usr/lib to be used (since CFLAGS is passed before LDFLAGS when invoking the linker, so that -L is getting there first), and you get a mismatch.

So it boils down to the code after the unless sdk.nil? or MacOS.clt_installed? check in Library/Homebrew/extend/ENV.rb -- that's what's putting that into CFLAGS. Looks like that's trying to work around a problem for some projects ("Needed because CC passes this to the linker and some projects forget to use the LDFLAGS explicitly") and causing this problem for others. Maybe this sort of fix needs to be activated on a per-formula basis.

But in the short term, I bet installing the Command Line Tools package would fix it for this user.

[samueljohn]

A good catch. Then, perhaps we remove that -L from the CFLAGS and I
have to check the individual formulae and update them. I see no other
clean way to go.

[tlberglund]

Installing CLT made it happen. brew doctor gives me a clean bill of health now as well. Thanks for your help! I'm no longer of any use in testing the fix, but it sounds like @jeffstyr has things well in hand.

[2bits]

So it boils down to the code after the unless sdk.nil? or MacOS.clt_installed? check in Library/Homebrew/extend/ENV.rb -- that's what's putting that into CFLAGS.

Well this is an XCode only problem according to the OP. So he should not satisfy clt_installed. So let's ignore that for a minute. He could only have satisfied sdk == nil. Why was his sdk==nil if he has XCode-4.3.3?

[jeffstyr]

No it's not that the unless check is failing, it's that it's succeeding -- the code it's protecting is the problem. Before the fix in #13037, the check was just unless sdk.nil?, and the compile was failing for basically everyone, which was the underlying cause of #13046. (Ignore my comment in #13046, I was on the wrong track.)

Basically, that code is putting linker flags into CFLAGS to try to work around bad Makefiles which forget to pass LDFLAGS to the linker, but it's breaking cases where the linker is getting both CFLAGS and LDFLAGS, with CFLAGS passed first.

[2bits]

Wait I get it.

[2bits]

Well I don't know it's succeeding to be honest. I don't have XCode only to test on.

[jeffstyr]

Right, succeeding means unless sdk.nil? is succeeding, which is equivalent to if sdk != nil. (The use of unless is really confusing in this case.)

The reason I'm pretty sure the check is succeeding is that I was getting the exact same error last week when the check was just unless sdk.nil? (and that check was succeeding on my system). I dug into what was going on in order to understand why #13037 had fixed it.

[2bits]

Ok so are -L/paths supposed to be in CFLAGS or is that a hack?

[jeffstyr]

I think it's a hack. The autoconf docs have some comments which back this up, regarding what flags go where.

[jacknagel]

Yes, it is a hack, and I do not like it. I made noise about this before the Xcode-only stuff was pulled, but the prevailing opinion seemed to be that doing it on a per-formula basis would quickly become a maintenance nightmare.

I would be interested to see just how bad it is, though. I would much prefer that the stuff in ENV.rb properly use CFLAGS, CPPFLAGS, and LDFLAGS if feasible. Perhaps an ENV.xcode_only helper could be designed that would add all these hacks at once for formulae that require it.

[jacknagel]

I also noticed recently that although ENV.setup_build_environment has code to avoid adding -L and -isystem flags when HOMEBREW_PREFIX is /usr/local (because it is already there by default), we end up adding these in ENV.macosxsdk anyway.

[2bits]

I like ENV.xcode_sdks if xcode_only? I am not in favor of -L in cflags.

[samueljohn]

I have a Xcode-only system and I can confirm this bug.
When I remove append_to_cflags "-L#{sdk}/usr/lib" from the ENV.macosxsdk method, ruby compiles.

Since removing stuff from the CFLAGS is harder than adding, we should remove this line and add it for formulae that actually need it!

@jeffstyr you are right. We had an issue that the unless sdk.nil? code was run for everyone when it should only be executed for xcode-only systems. That broke postgres, valgrind and a few other. But that has been fixed already.

Right, succeeding means unless sdk.nil? is succeeding, which is equivalent to if sdk != nil. (The use of unless is really confusing in this case.) -- @jeffstyr

I am not a fan of unless at all but its the ruby style I guess :-)

@jacknagel, @2bits @mistydemeo:
Well, I have to look how many formulae actually need that "-L#{sdk}/usr/lib" to be added to CFLAGS. Perhaps it is clearer to do this for each formula than to call ENV.xcode_handle_special_cases where new formula writer don't really know what's going on in that call.

I consider a -L in the CFLAGS a hack, too. So let's pretend we live in a better world and remove this flag and punish formulae that need it with an extra verbose line like:

ENV.append_to_cflags MacOS.sdk_path/'usr/lib' if MacOS.xcode_only?

Note that we don't have xcode_only?, yet. But it's a good idea.

[samueljohn]

oh and this issue should be flagged xcode_only.

[2bits]

yes that's better to add a specific ENV.append 'CFLAGS' to the formula rather than an unclear ENV.handle.special.case.

[jeffstyr]

And brew --config should report whether CL Tools is installed. (I think it doesn't currently.)

[samueljohn]

@jeffstyr it does: CLT: N/A means that it's not there (as on my Xcode-only setup.

[jeffstyr]

Ah. That output doesn't show up for me, because of the check in:

    config_s << "CLT: #{describe_clt}\n" if MacOS.xcode_version.to_f >= 4.3

Since "N/A" is valid output anyway, it should probably report that for the < 4.3 case. Side issue though.

[2bits]

@samueljohn Are you working on a core patch to fix CFLAGS? If that's going to take a few days, what's the patch for the ruby formula in the meantime?

[jacknagel]

@jeffstyr We omit it for older Xcode because while it does have a defined version, it was not a separate package.

[jeffstyr]

Right, since it's not relevant for older Xcode versions, I was thinking CLT: N/A would express that well.

[jacknagel]

N/A in this context usually means "not installed" or "not found", and since it's not relevant to debugging we just omit it.

[jeffstyr]

Okay. (Although "N/A" actually means "not applicable", which fits the < 4.3 case; in those other cases it should probably actually say "not installed" or "not found", since it actually is applicable information in those situations.)

@2bits I'm not sure if there is any non-convoluted temporary fix that can be made to the ruby formula itself; I think the easiest workaround is just to install the CLT (or, just comment out this line of code locally, which is basically the long-term fix anyway).

On the core topic (since we are looking at this area of the code), clt_installed? is defined as:

not clt_version.empty? or dev_tools_path == Pathname.new("/usr/bin")

If you think through the boolean-ness, the clt_version check seems extraneous; with that, you'll get true in the case in which the package is installed but the tools are somehow not in /usr/bin, and really you should go ahead and get false in that case. I think the check should just be dev_tools_path == Pathname.new("/usr/bin"); that's testing what the callers of this are really getting at. (Which makes the method a bit misnamed. It really means something like tools_at_standard_paths?.) That would save a bunch of calls out to pkgutil.

[jacknagel]

N/A also means "not available", which is usually how I read it in the config output.

I agree about clt_installed?, I think. Will look at it harder later.

[adamv]

Pulled the likely fix; please brew update.

[samueljohn]

@jeffstyr please check and report if there are still issues. Thanks for reporting!

[jeffstyr]

@samueljohn I just debugged it, it was @tlberglund who reported it. (It wasn't broken for me currently, since your fix last week that added the MacOS.clt_installed? check.) But, you are welcome! :)