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

R depends on gettext to fix build issue. #18198

Closed
wants to merge 1 commit into from

Conversation

manphiz
Copy link
Contributor

@manphiz manphiz commented Mar 3, 2013

  • Otherwise it fails to link due to "missing -lintl".

Unfortunately the failed build log was erased by the successful build. It shouldn't happen.

@Sharpie
Copy link
Contributor

Sharpie commented Mar 3, 2013

I came across this yesterday---R doesn't actually need gettext. This is happening if our cairo build gets drug in instead of the cairo libraries proved by :x11.

So, we have to find a way to ensure depends_on :x11 takes precedence.

@jacknagel
Copy link
Contributor

In most cases we always use our cairo, to prevent situations where something has transitive dependencies on cairo from Homebrew and x11.

@Sharpie
Copy link
Contributor

Sharpie commented Mar 3, 2013

In this case, R needs to be built against X11 as there is a small galaxy of R packages that expect it to be there.

So, it looks like we could solve this by doing something like:

depends_on 'cairo'
depends_on :x11

But, that seems redundant.

@jacknagel
Copy link
Contributor

That's how we've been doing it; see gtk+, for example.

@Sharpie
Copy link
Contributor

Sharpie commented Mar 3, 2013

Also, another thing to note is that when I observed this issue the problem wasn't that gettext couldn't be found, the R configure script was adding something to the link path:

-L/usr/local/Cellar/gettext/<version>/lib

The problem was that <version> had been pruned by brew cleanup. I'll try to run a test to see if it will use /usr/local/opt references now, if so that the real problem is just a stale link path from back in the day when keg_only was fragile. If not, then this is a more general brew upgrade breaks things problem that we have had since forever.

@MikeMcQuaid
Copy link
Member

R needs to be built against X11

If it's installed and/or with the option specified. When we don't require X for a package to function we shouldn't require it by default (@mxcl agreed with me on this) as users don't have it on 10.8.

@Sharpie
Copy link
Contributor

Sharpie commented Mar 3, 2013

If it's installed and/or with the option specified. When we don't require X for a package to function we shouldn't require it by default.

R is one of those cases where X11 is really not optional. The official R build ships with X11 support and there is a metric ton of R packages out there that use it.

Switching it off by default would result in a stream of confused statisticians wandering around the issue tracker asking why their code no longer works.

@mistydemeo
Copy link
Member

Cairo isn't in R's dependency tree. Where is it picking it up from?

@manphiz
Copy link
Contributor Author

manphiz commented Mar 3, 2013

On Mar 3, 2013 2:44 PM, "Misty De Meo" notifications@github.com wrote:

Cairo isn't in R's dependency tree. Where is it picking it up from?

I was wondering the same thing. Shouldn't superenv have it taken care of?


Reply to this email directly or view it on GitHub.

@Sharpie
Copy link
Contributor

Sharpie commented Mar 3, 2013

I think it pulls the Cairo info from pkg-config.

@jacknagel
Copy link
Contributor

Shouldn't superenv have it taken care of?

cairo isn't keg-only on 10.8, so it's pkg-config file can be located through HOMEBREW_PREFIX/lib/pkgconfig, which is baked into the pkg-config binary.

@MikeMcQuaid
Copy link
Member

R is one of those cases where X11 is really not optional.

It still sounds optional to me. Our XQuartz handling on 10.8 is not nice; you are halted in your tracks and told to go and install some software from a DMG. Feel free to print a large warning or whatever but it should compile even if you don't have X11 installed.

@Sharpie
Copy link
Contributor

Sharpie commented Mar 3, 2013

The wierd part is that I can no longer re-produce this on the box that was exhibiting the issue after ripping cairo out and reinstalling. It may be that this behavior only happens on machines where the cairo/gettext installation is old and crusty.

@Sharpie
Copy link
Contributor

Sharpie commented Mar 3, 2013

It still sounds optional to me. Our XQuartz handling on 10.8 is not nice; you are halted in your tracks and told to go and install some software from a DMG.

But if you are an R user who expects X11 support to be there (the majority of R users) then you are halted in your tracks (possibly months later when something mysteriously fails) and have to re-install R --with-x. From my experience on this project, many people ignore loud nasty warnings unless they actually halt the build process.

Furthermore, even if we drop :x11 and include cairo instead, cairo also depends_on :x11 by default.

@manphiz
Copy link
Contributor Author

manphiz commented Mar 4, 2013

On Sun, Mar 3, 2013 at 3:25 PM, Charlie Sharpsteen <notifications@github.com

wrote:

The wierd part is that I can no longer re-produce this on the box that was
exhibiting the issue after ripping cairo out and reinstalling. It may be
that this behavior only happens on machines where the cairo/gettext
installation is old and crusty.

​Reinstalling cairo doesn't fix it for me: Building r​ still fails due to
"missing -lintl".

Reply to this email directly or view it on GitHubhttps://github.com//pull/18198#issuecomment-14358107
.

* Otherwise it fails to link due to "missing -lintl".
@manphiz
Copy link
Contributor Author

manphiz commented Mar 4, 2013

Changed to depends on 'cairo' as suggested.

@benjaminarai
Copy link

Adding "cairo" worked for installation but now trying to install any R package results in the same "missing -lintl" message.

@ghost
Copy link

ghost commented Mar 8, 2013

To get package installation to work, I did a brew link gettext --force. I can now install packages, but I worry that it might get me into trouble down the road. Anyway, so far, so good.

@manphiz
Copy link
Contributor Author

manphiz commented Mar 8, 2013

So now it looks like the real fix should be in pkg-config, which should pick up cairo in X11/XQuartz instead of the one in Homebrew.

@jacknagel
Copy link
Contributor

So now it looks like the real fix should be in pkg-config, which should pick up cairo in X11/XQuartz instead of the one in Homebrew.

Nope, it is our policy to always use Homebrew's cairo to keep things consistent. This is why, for example, gtk+ depends on Homebrew's cairo even though it depends on X11 as well.

@manphiz
Copy link
Contributor Author

manphiz commented Mar 8, 2013

On Fri, Mar 8, 2013 at 1:07 PM, Jack Nagel notifications@github.com wrote:

So now it looks like the real fix should be in pkg-config, which should pick up cairo in X11/XQuartz instead of the one
in Homebrew.

Nope, it is our policy to always use Homebrew's cairo to keep things consistent. This is why, for example, gtk+ depends on
Homebrew's cairo even though it depends on X11 as well.

But AIUI the problem with homebrew's cairo is that it is keg_only,
which cannot be picked up by external R packages unless it is
force-linked and have higher precedence in $PATH, or its paths are
manually added to $DYLD_LIBRARY_PATH.

Also, just curious, originally homebrew tried to avoid duplication
from system stuff, and I guess XQuartz should have libraries with
versions high enough, so why not make use of them now?

Re
ply to this email directly or view it on GitHub.

@jacknagel
Copy link
Contributor

and I guess XQuartz should have libraries with versions high enough, so why not make use of them now?

Not everyone has the most up-to-date XQuartz release. Some software in Homebrew won't build with older cairo. We started using only Homebrew's cairo to maintain consistency, and avoid situations where things are transitively linked against two different versions of cairo.

@manphiz
Copy link
Contributor Author

manphiz commented Mar 8, 2013

Not everyone has the most up-to-date XQuartz release. Some software in Homebrew won't build with older cairo. We started using only Homebrew's cairo to maintain consistency, and avoid situations where things are transitively linked against two different versions of cairo.

Now I see.

For this particular problem, I'd assume the proper fix is still #18243, no?

@jacknagel
Copy link
Contributor

Thinking about it, yes.

I just want it to be clear why the current behavior exists and is the right thing for most situations.

@manphiz
Copy link
Contributor Author

manphiz commented Mar 8, 2013

On Fri, Mar 8, 2013 at 2:02 PM, Jack Nagel notifications@github.com wrote:

Thinking about it, yes.

I just want it to be clear why the current behavior exists and is the right thing for most situations.

Cool. Thanks for the clarification.


Reply to this email directly or view it on GitHub.

@jacknagel
Copy link
Contributor

Patch to wrangle pkg-config here: https://github.com/jacknagel/homebrew/compare/pkgconfig

@jacknagel
Copy link
Contributor

Pushed; pkg-config should no longer be able to pick up non-deps under superenv. Hopefully that means this is resolved.

@jacknagel jacknagel closed this Mar 9, 2013
@manphiz manphiz deleted the r-build branch March 9, 2013 21:22
@manphiz
Copy link
Contributor Author

manphiz commented Mar 9, 2013

@jacknagel retried building r and worked. Thanks for your work.

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

6 participants