Skip to content

Loading…

Not all versions of autoreconf respect `ACLOCAL_PATH` or `ACLOCAL_FLAGS` #10824

Closed
Sharpie opened this Issue · 25 comments

6 participants

@Sharpie

In the past, it was a common practice to encode the path to non-standard aclocal directories, such as #{HOMEBREW_PREFIX}/share/aclocal or the aclocal directory of a keg-only formula by setting them as an explicit part of the aclocal call:

ENV['ACLOCAL'] = "aclocal -I#{HOMEBREW_PREFIX}/share/aclocal #{gettext.share}/aclocal

However, these declarations were removed in commits like 56771ee and e8bb54b based on the assumption that we now add these directories to ACLOCAL_PATH so everything should be good.

However, it appears that older versions of autoreconf, from Xcode 4.2 and lower, do not respect either ACLOCAL_PATH or ACLOCAL_FLAGS, so explicitly setting ACLOCAL appears to be our only option.

We need to either:

  • Re-introduce the calls to ENV['ACLOCAL']

  • Bite the bullet and duplicate autotools for every Homebrew installation, not just Xcode 4.3 and above so that we can use ACLOCAL_PATH and stop abusing ACLOCAL.

  • Find some other way around the problem.

@Sharpie

@mxcl @adamv @mikemcquaid @mistydemeo @jacknagel

Any thoughts on the best way to proceed with this one?

@mistydemeo

I think it's time to bite the bullet and introduce a keg-only autotools dupe on older Xcodes. I'd see the harm as being minimal, versus the benefit of being able to standardize how things like ACLOCAL are dealt with on all versions of Xcode.

@jacknagel

It's certainly not going to get any "better" in the sense that things will start requiring newer versions of autotools in the future anyway. And it's probably going to be less of a burden to do everything the same on all platforms rather than maintaining a special hack for < 10.7. So yeah, I don't mind duping them.

@Sharpie

Pulling the if MacOS.xcode_version >= "4.3" guard out of the fuse4x formula does indeed solve issue #10853. It seems like duplicating the autotools is pretty safe as they will be keg_only when the Xcode version is less than 4.3.

@adamv @mikemcquaid @mxcl

Any arguments against duping autotools for all installations?

@mikemcquaid
Homebrew member

I was against it but no longer am. If it's got removed then we may as well just duplicate it everywhere.

@adamv

Even though I'm still on Snow Leopard, my current thinking is "make things more convenient for the future", ie, supporting Lion (and Mountain Lion) as first-class, letting Snow Leopard get a little crufty, let Leopard users pitch in if they want to. But that's just me.

@Sharpie

Well, "first class" support would be setting these things through ACLOCAL_PATH which is what we currently do. The problem is that this doesn't work for Xcode 4.2 and lower---so we either have to let some cruft leak into the formulae or depend on modern versions of autotools everywhere.

@Sharpie

Allright, removed all the if MacOS.xcode_version >= "4.3" guards I could find in #10901.

@Sharpie

@mxcl @adamv @mikemcquaid @jacknagel @mistydemeo

I'm going to pull the trigger on #10901 on Thursday and dupe this for everyone unless anyone comes up with an objection or alternate plan.

@adamv

I (oddly?) have no opinion on this.

@jacknagel

I'm good with that.

@mikemcquaid
Homebrew member

Works for me.

@mxcl
Homebrew member

There will be duping pain.

Seems less effort and more likely to not break stuff to just make ENV['ACLOCAL'] have the additional flags as well.

@Sharpie

There will be duping pain.

Right. But if we set ACLOCAL_PATH in build.rb and users set ENV['ACLOCAL'] in their formulae and different versions of autotools give different precedence to each setting there will be maintenance pain caused by the divergent behavior.

Seems less effort and more likely to not break stuff to just make ENV['ACLOCAL'] have the additional flags as well.

Fair enough, although I would rather handle this at a higher level than individual Formulae. Should we set the following in build.rb after adding keg-only formulae to ACLOCAL_PATH?

ENV['ACLOCAL'] = 'aclocal ' + ENV['ACLOCAL_PATH'].split(':').map {|p| '-I' + p}.join(' ')
@mxcl
Homebrew member

I was thinking more ENV.rb this is the correct place to modify the build environment and will then show up in brew --env.

We should make setting ACLOCAL in a formula audited and not condone it.

I've seen enough issues where build stuff hardcodes /usr/bin/automake or whatever, however I'm sure we could work around this on a formula by formula basis. So I leave the decision to you, you know what you are doing here.

Dupe issues are the hardest to diagnose though.

@Sharpie

I was thinking more ENV.rb this is the correct place to modify the build environment

The problem is that we modify ACLOCAL_PATH extensively in build.rb by prepending values. With this behavior, I don't see any way of faithfully setting ACLOCAL with -I flags unless it is done in build.rb right after ACLOCAL_PATH has been set.

Off-topic, but perhaps modifications related to keg-only dependencies should eventually be moved into ENV.setup_build_environment instead of being done in build.rb?

@mxcl
Homebrew member

The problem is that we modify ACLOCAL_PATH extensively in build.rb by prepending values. With this behavior, I don't see any way of faithfully setting ACLOCAL with -I flags unless it is done in build.rb right after ACLOCAL_PATH has been set.

Well this is only for keg-only deps (AFAIC see), we'd need to also modify ACLOCAL there too.

Off-topic, but perhaps modifications related to keg-only dependencies should eventually be moved into ENV.setup_build_environment instead of being done in build.rb?

Well the setup_build_environment is the base environment. The keg-only mods are the second layer of modification and belong in build IMO.

@mxcl
Homebrew member

Ideal solution: figure out which version of autotools supported ACLOCAL_PATH etc. then set ACLOCAL before that and ACLOCAL_PATH after.

Duping autotools on old platforms is probably a bad idea. We don't test the old-platforms that much anymore. Better to try and make behavior as close to before the brew-tap changes I made as possible.

@adamv

I'm willing to try running with newer autotools on Snow Leopard, if we want "always use newer ones" as the going forward position. Esp. since newer Xcode needs them anyway.

@mxcl
Homebrew member

Being more pragmatic, I believe my earlier dupe concerns don't matter much for autotools. Maybe. Because they are designed for it.

@adamv

The status quo is working out for me on Lion + Snow Leopard. I'm not opposed to using newer versions on Snow Leopard, since they all brew pretty fast, but pull request or whatever. Closing due to fuse4x seeming to work OK how it is currently.

@adamv adamv closed this
@Sharpie

Sounds good to me.

@mxcl
Homebrew member

My superenv branch has a solution for this, hopefully. Though superenv is currently for Xcode >= 4.3, it can be made with older configurations quite easily, I just can't test.

@mxcl
Homebrew member

I'm still worried about this bug. Would be nice to know what aclocal version doesn't support ACLOCAL_PATH and cater to it. I can't find such information on the Internets.

@Sharpie

Would be nice to know what aclocal version doesn't support ACLOCAL_PATH

As I recall, the unfortunate answer to this question is "all the versions that ever shipped with XCode". Don't know which release caused aclocal to start paying attention to this environment variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.