Turn off link time optimization for ponyc builds

[SeanTAllen]

By building bottles with lto aka link time optimization on, user's must
run the same XCode as the bottle was built with. Example scenario where
this causes issue:

User installs XCode in July 2017. Gets XCode 8.x
User installs Ponyc in October 2017. Gets a version built with XCode 9.x

Because of the mismatch in xcode versions and lto being on, programs
won't compile.

This change results in a drop in overall performance for compiled Pony
programs but improve basic usability. We, the Pony core team, discussed
the issue and decided that this was a better way to build when using
homebrew.

In order to get peak performance, you already need to install ponyc from
source, so, this makes sense as a change.

• Have you followed the guidelines for contributing?
• Have you checked that there aren't other open pull requests for the same formula update/change?
• Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
• Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

---

[ilovezfs]

How significant is the performance difference?

Using pour_bottle? do might be worth considering instead since we really only support using the latest Xcode anyway.

[SeanTAllen]

@ilovezfs we don't use LTO anywhere other than OSX. There's a performance difference but its small compared to other performance loses you get by not building from source. We think this is a good trade-off for increased usability to get people using Pony. Anyone who is going to be serious about Pony and performance is going to need to build from source to get optimum performance.

Note: Without this change, updating XCode will also break an existing Pony install due to the bitcode differences with LTO. We've come to the conclusion that LTO is an optimization that should only be turned on by the user themselves given the fragility it can introduce.

It's the opinion of the Pony core team that this change is quite desirable given the profile of users' who install Pony via homebrew.

[ilovezfs]

And you're aware of what pour_bottle? do does?

[ilovezfs]

I'm pretty 👎 on the idea of making changes that hurt the performance for people who are using the supported configuration (using the lastest Xcode) for the sake of people who are using an unsupported configuration (using something other than the latest Xcode). Also, note that brew install --build-from-source does an -march=native build.

[SeanTAllen]

@ilovezfs I understand what pour_bottle does. I understand your performance concerns. we wanted to avoid this but came to the realization that:

1. basically no one installing via homebrew understands whats LTO is.
2. this will lead to broken installs

Let's take this example:

• I install ponyc from homebrew.
• Some time later, a new version of xcode comes out
• I update my xcode

The above will break the ponyc install if this PR isn't installed. This isn't just about using an "unsupported" configuration. It's about a working configuration being broken silently on you by upgrading XCode. That's a really poor user experience.

The standard user profile of folks installing ponyc via homebrew are folks who aren't aware of the existence of LTO. They come to us for support because this ends up broken. Some do anyway. We suspect others just give up. All our user surveys show that "advanced" ponyc users on OSX install from source, new folks who would be tripped up by this use homebrew.

Re: performance consideration: if you want maximum Pony performance, you have to build from source, so the small performance hit from this is kind of moot. If you care about performance, we already tell folks to not install via homebrew.

[ilovezfs]

It's about a working configuration being broken silently on you by upgrading XCode.

Does this affect upgrades like 8.2 -> 8.2.1 or only upgrades like 8.3.3 -> 9.0?

[SeanTAllen]

@ilovezfs any upgrade where the LLVM bitcode version change to be incompatible with one another. As far as I know thats 8.3.3 -> 9.0 but I can't guarantee that 8.2 -> 8.2.1 wouldn't. I'm not familiar enough with how. I don't believe that 8.2 -> 8.2.1 breaks bitcode versions based on how Apple does versioning but I can't guarantee that.

Re: this PR, after explaining the problems with having LTO on for OSX, I realized that this PR isn't the best way to go about this. I'm going to close this and we are going to revert on the Ponyc side to have LTO off for OSX. It's off for all platforms except OSX. I was the person who turned it on by default for OSX several months back. I realize now that by having it on by default, we are leading people down this path of possible confusion and breakage. It would be much better for us to leave LTO off and as part of our "how to get optimal performance" documentation, that we tell people how to turn on LTO and the implications that has for breaking on them when the OSX linker changes. We can document how and when the OSX linker can change and they can go into this aware.

This PR would have left the same problem I detailed in my previous message for anyone who installs from source without understanding what LTO is.

I'm going to close this PR, we'll make updates on the ponyc side and move forward with that as our default.

I'd like to note that the pour_bottle? that is the ponyc formula isn't doing what it was placed in there to accomplish and can probably be removed.

  pour_bottle? do
    reason <<-EOS.undent
      The bottle requires Xcode/CLT 8.0 or later to work properly.
    EOS
    satisfy { DevelopmentTools.clang_build_version >= 800 }

Without the LTO issue, older versions of DevelopmentTools.clang_build_version would work. With LTO, that isn't sufficient to prevent broken installs.

To recap, this problem was created several months ago by me turning on LTO by default for OSX. That was a mistake on my part. It an advanced feature that shouldn't be on by default. It can lead to broken installs. It should only be on if the user understands the ramifications. I realize now that making this change on the homebrew side is the wrong place to do it. I'm closing this PR and going back to how we previously handled LTO on the ponyc side.

Sorry for taking up your time @ilovezfs.

[ilovezfs]

@SeanTAllen no worries. Your analysis sounds correct to me. If you do want to do this in the Homebrew formula only, I won't object since you've clearly thought a lot about this :)

Another option would be

if build.bottle?
  system "make", "config=release", "lto=no", "destdir=#{prefix}", "install", "verbose=1"
else
  system "make", "config=release", "destdir=#{prefix}", "install", "verbose=1"
end

to allow lto for people who are doing --build-from-source.

Unless you're planning on doing an imminent release, we should probably revision bump and disable lto to save you the hassle of a full release and so that you can take your time with the ponyc side of it.

[SeanTAllen]

@ilovezfs I was planning on doing a release soon anyway. I'm going to do one today. You'll have a PR to update homebrew to 0.20.0 of ponyc in the next 12 hours or so.

[ilovezfs]

@SeanTAllen Regarding bitcode compatibility, it's worth noting http://blog.llvm.org/2016/12/llvms-new-versioning-scheme.html says

Bitcode Compatibility
Before LLVM 4.0.0, the Developer Policy specified that bitcode produced by LLVM would be readable by the next versions up to and including the next major release. The new version of the Developer Policy instead specifies that LLVM will currently load any bitcode produced by version 3.0 or later. When developers decide to drop support for some old bitcode feature, the policy will be updated.

so the problem may be less likely going forward than you're thinking.

[SeanTAllen]

@ilovezfs if it stops happening, i will be thrilled. in the meantime, we'll go the safe route.