qt: use 4.8 branch snapshot for stable on 10.9.

[MikeMcQuaid]

Would be nice if this could be done another way but I think it's better than making users use --HEAD until Qt fixes their mess (in January):
http://permalink.gmane.org/gmane.comp.lib.qt.devel/13812

[mistydemeo]

4.8.5-157da3?

[MikeMcQuaid]

We would need to bump the 10.7 and 10.8 bottles to that too if that's the case and I think it's better the majority of versions are correct :(

[mistydemeo]

@cliffrowley How close are the patches to making their way into the tree?

[MikeMcQuaid]

@mistydemeo I had a look at them a few days ago. They've been reviewed once but not again. We can always update this at some point too; I just want to fix the currently crappy user-experience here.

[jensenb]

@MikeMcQuaid That's not completely accurate. What patches did you check?

Here are the currently required changes, all of which have been approved but not merged:
https://codereview.qt-project.org/#change,70444
https://codereview.qt-project.org/#change,70439
https://codereview.qt-project.org/#change,71983
https://codereview.qt-project.org/#change,70929
https://codereview.qt-project.org/#change,70930

The WebKitSystemInterface for Mavericks is under discussion and might not be included.

If you are going to change the formula and version, I think it would be wise to also update the patches. Some faulty changes got lumped in with all of the Qt changes in #23793 and it would be good to use the approved changes from the Qt guys and not the patches that are currently in the formula.

[MikeMcQuaid]

@jensenb We won't change the version but I'm happy to change the patches.

[MikeMcQuaid]

@jensenb There's currently 10 patches in the Qt formula and you've only listed 5 there: how come? Also, could you update your Gists and paste the new URLs in here (and the corresponding Gerrit links)? Thanks.

[cliffrowley]

@mistydemeo I'm not a Qt contributor / maintainer so I have no idea, sorry!

I only got involved in the first place because Qt was broken on Mavericks and nobody else was stepping up! ;-)

@MikeMcQuaid I'm happy to do this update if you like, if @jensenb gives us a list of patches that should be included?

Edit: actually it might be better if you guys take care of it in this instance, given the necessity to get rid of --HEAD etc. I'm a little out of my depth here.

[jensenb]

So here are the updated patches in the same order as above, plus the WebKitSystemInterface change that is necessary at the moment, but may not make into 4.8:

 [
      # Change Change I31ad9a7a: Fix WebKit TimerHeap implementaiton to work with more standard libraries
      # (https://codereview.qt-project.org/#change,70444)
      'https://gist.github.com/jensenb/aafb2c2d1e0fcce2994f/raw/dc66754ff9131858d653655fc7c9966abde866a5/Change_I31ad9a7a',
      # Change Ied51c868: WebKit Nullptr type fixes when using libc++
      # https://codereview.qt-project.org/#change,70439
      'https://gist.github.com/jensenb/aafb2c2d1e0fcce2994f/raw/61d7af7095dbc4612765c961fca1cf6f333fa88d/Change_Ied51c868',
      # Change Idd350a9d: Correct WebKit build path settings
      # (https://codereview.qt-project.org/#change,71983)
      'https://gist.github.com/jensenb/aafb2c2d1e0fcce2994f/raw/1ba52b15ddc02431b9773221c399d6ea2297cf93/Change_Idd350a9d8',
      # Change Ieb30c115: Backported fix for WebKit libc++ support on OS X Mavricks
      # https://codereview.qt-project.org/#change,70929)
      'https://gist.github.com/jensenb/aafb2c2d1e0fcce2994f/raw/a47555eead9d0a068fc329c4a0c04c834fc5b262/Change_Ieb30c115',
      # Change Iaedaff7c: Enable building with clang / libc++ on OS X 10.9 Mavericks
      # (https://codereview.qt-project.org/#change,70930)
      'https://gist.github.com/jensenb/aafb2c2d1e0fcce2994f/raw/85a60f149759a9805e9ba0c6a182842317adff37/Change_Iaedaff7c',
      # Change I780062ce: Add WebKitSystemInterface for Mavericks
      # (https://codereview.qt-project.org/#change,70441)
      'https://gist.github.com/jensenb/aafb2c2d1e0fcce2994f/raw/85228f1358c8f9c33049209bb92ccf6aff914c53/Change_I780062ce5971',
    ]

Its 5 changes now because the dev who originated some of these changes went a little overboard with his review requests and created a request for each individual file, instead of grouping things by logical change. So there has been some regrouping of the changes, and some things got removed because they were incorrect (some of the nullptr stuff etc..).

[jensenb]

Hold off testing the updated patches for a moment, I am seeing the CLucene linker issue again, that I don't see when I build from my qt tree manually, I am investigating.

[MikeMcQuaid]

Do we still need https://codereview.qt-project.org/#change,62261?

[jensenb]

I don't think that change is necessary for building, I have not been including it in my builds.

I believe I have also isolated the problem. I was building without setting the environment variable:

CXXFLAGS="-fvisibility=hidden"

This is set by formula currently. When this is set, it causes CLucene linker errors, that are otherwise not present without it being set. So two questions:

• What is the purpose of setting this variable for building Qt?
• If necessary / recommended, shouldn't this be added to build flags in the Qt build configuration?

[MikeMcQuaid]

It's require for parts of KDE and various other dependencies (e.g. Phonon) if I recall correctly.

I think I may just pull this PR as-is and then we can update the snapshot and/or bottle when these changes are merged into Qt proper. Thanks for all the legwork there, though.

[jensenb]

Sure that is probably the safest bet right now.

I would like to take the initiative to fix thing longer term. Do you have more specific information on the dependencies that require the hidden visibility compiler flag? I looked and KDE is no longer in homebrew, and the phonon module builds just fine on my system without that flag.

If it is really necessary then I would open a change request with Qt and add it to the mac qmake spec files. Otherwise I think ti should be removed, since it prevents Qt from building CLucene.

[MikeMcQuaid]

I don't; you could delve through the Git logs for it. Do you mean CLucene itself or QtCLucene (which is just, if I recall correctly, to index help files).

[jensenb]

With visability=hidden libQtCLucene doesn't like due to missing missing symbols from the std lib:

Undefined symbols for architecture x86_64:
  "std::string::_Rep::_M_destroy(std::allocator<char> const&)", referenced from:

I don't really understand why the error goes away when the flag is left out...

[jensenb]

I figured out the problem, it isn't due to the -fvisability=hidden compiler flag via the CXXFLAGS environment variable after all. It turns out that just setting the CXXFLAGS variable to any valid compiler flag causes the QtCLucene build to drop some of the compiler flags from the user specified qmake spec file during parts of its compilation, leading to -stdlib=libc++ being dropped for parts of the compilation, and thus the typical linker error messages when you mix libc++ and libstdc++ code.

[jensenb]

By the way, I am fairly certain this is not needed anymore:

ENV.append "CXXFLAGS", "-fvisibility=hidden"

Qt automatically adds this flag where appropriate, defined in mkspecs/common/gcc-base.conf, which is used by both clang and gcc.

[MikeMcQuaid]

Ok. Feel free to submit that with your next PR too. Cheers.