Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qt5 5.7.0 #2087

Closed
wants to merge 2 commits into from
Closed

qt5 5.7.0 #2087

wants to merge 2 commits into from

Conversation

LRFLEW
Copy link
Contributor

@LRFLEW LRFLEW commented Jun 17, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same formula update/change?
  • Have you built your formula locally prior to submission with brew install <formula> (where <formula> is the name of the formula you're submitting)?
  • Does your submission pass brew audit --strict --online <formula> (after doing brew install <formula>)?

I know that major version changes to qt5 have been more involved in the past, such as Homebrew/legacy-homebrew#50234. However, I saw that there was no open PR for this, so I went ahead and made one.

@UniqMartin
Copy link
Contributor

Thanks! You beat me to it this time! 😉 I was still in the process of building things locally and performing some tests. Anyway, the build is re-queued with sufficient buffer before it times out. Let's what our build bot has to say about this new major release … I guess we'll know more in about three hours. ⌛

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jun 17, 2016

One of the builds (mavericks) has already failed. It failed on the brew linkage --test pyqt5 call. Along with complaining about not finding QtWebEngineCore and QtWebEngineWidgets (which I can see on my local machine with the install I did of this before making the PR), it's complaining about not finding Enginio.framework, which was officially removed from the final release packages with 5.7 (source). This means that the pyqt5 package probably needs to be edited to no longer rely on Engine by default.

@UniqMartin
Copy link
Contributor

First, let's wait until the other two builds finish before taking any action or jumping to any conclusions.

One of the builds (mavericks) has already failed.

Unfortunately, this is not looking good. Our Mavericks box is frequently the fastest, but this was way too fast for a Qt build and given the other errors, it makes me think that—for reasons yet unknown to me—the whole QtWebEngine module wasn't built. This would be consistent with the observed build time.

This means that the pyqt5 package probably needs to be edited to no longer rely on Engine by default.

Since you already have a working Qt 5.7.0 locally, can you try building pyqt5 from source? It normally automatically adapts to the frameworks available in a given Qt installation, thus if there is no other breakage, it should be sufficient to bump the formula revision of pyqt5 (add a revision 1 line after the first sha256 and put this in a separate commit titled pyqt5: revision for qt5 in this PR's branch).

@UniqMartin
Copy link
Contributor

When you make changes to the branch, please also remove these lines (currently 53–55) in qt5.rb. OS X 10.8 is now officially the oldest supported OS X release, so this comment no longer makes sense.

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jun 17, 2016

pyqt5 appears to be working after compiling from source on my local machine. I'll go ahead and make the changes now, and let the test run to see if it fails again.

@DomT4 DomT4 added the in progress Stale bot should stay away label Jun 18, 2016
@DomT4
Copy link
Contributor

DomT4 commented Jun 18, 2016

I'm always hopeful Qt has made significant strides in build time reduction, but yeah, from over 2 hours to 50 minutes with those failures seems deeply suspicious.

The generated Mavericks bottle is also only 46.78MB compared to 91.31MB for El Cap and 91.08MB for Yosemite. 91MB is about consistent with the bottles for recent Qt5 releases.

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jun 18, 2016

Yeah, there's something going wrong with the web engine build on Mavericks. I checked the bottles myself, and the only missing files are related to QtWebEngine, QtWebEngineCore, QtWebEngineWidgets, and QtWebView. I'd like to see the output of the ./configuration call in the build on Mavericks. I'll see if I know anybody online still on Mavericks to see if I can figure something out.

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jun 18, 2016

Hmm. While looking up to see if the Web Engine had any additional requirements on top of Qt itself, I found this: http://doc.qt.io/qt-5/qtwebengine-platform-notes.html#os-x

It says that 10.9 (Mavericks) should be able to build the Web Engine, though the page does list some caveats that I'm not sure the build server meets. It says that it needs Xcode 5.1 or later, which seems like it might be a newer version than the one that came with Mavericks. Earlier on the page, it also mentions requiring "Bison, Flex" and "GPerf". From what I can tell, Mavericks (with the command-line tools installed) should have all of those, but I wonder if there was a version difference for one of those libraries between 10.9 and 10.10 that's causing the issue.

@UniqMartin
Copy link
Contributor

Yes, there's something weird going on with the QtWebEngine build on Mavericks. This much is clear.

I'd like to see the output of the ./configuration call in the build on Mavericks.

I re-scheduled the build on our machines and collected the configure logs. Have replaced the somewhat random temporary directory with @TMP@, but otherwise these are the full logs. Maybe that helps finding the cause: https://gist.github.com/UniqMartin/2885ddf3bed0e98715d376689380f0bf

Hmm. While looking up to see if the Web Engine had any additional requirements on top of Qt itself, I found this: http://doc.qt.io/qt-5/qtwebengine-platform-notes.html#os-x

None of the stuff listed there sounds like it should create a problem on Mavericks.

It says that 10.9 (Mavericks) should be able to build the Web Engine, though the page does list some caveats that I'm not sure the build server meets.

Shouldn't be the issue. All our machines are updated to the latest Xcode still supported on that OS X release, i.e. for 10.9 this is Xcode 6.2 and the matching Command Line Tools (see 00.config-* in Gist).

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jun 18, 2016

I've scanned over the logs you provided and didn't find anything incredibly useful unfortunately. I was hoping that the configuration output specified what modules were being compiled, but it only names a few in that output. I'm worried the answer I'm looking for is from the output of the make command, but I don't personally want to scan through an hour of logs. Maybe doing something like "grep error" would reveal something.

The weren't many differences in the configurations, but one difference did stand out. 10.10 and 10.11 differed from 10.9 with the support for c++1z and AVX512, but neither of those should be an issue. The other difference is that the 10.9 config had the "Using gold linker" option turned on. This had me really confused, as my research on the "gold linker" says that it only supports ELF formats. I don't know if this is a cause for the issues, but it's the only thing I see so far.

@UniqMartin
Copy link
Contributor

The C++1z and AVX stuff is to be expected, as that's related to what the compiler supports and of course the slightly older Clang distributed with Xcode 6.2 doesn't support everything the one from Xcode 7.3.1 supports.

The full build log might help, but it is a bit tricky to capture on our test bot because the test bot makes sure to automatically clean up after itself. Maybe that's something we need to improve so that the logs of a build can be more easily retrieved in case there is no obvious error or not in the last few lines that are printed on a failed build. (But here it doesn't fail, it just skips building one part of the library.)

I agree that the “Using gold linker” stuff is really suspicious, as it doesn't make sense on OS X. To me this sounds like a bug that was introduced in the configure script or the related configure test. Not sure if this is the cause, but if it is, we could just pass -no-use-gold-linker to configure to disable the automatic detection. Would you mind trying that?

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jun 19, 2016

Well, adding the -no-use-gold-linker didn't fix it. I'm kind of at a loss here. The only thing I can think to do right now is to check the Makefile to see if it's a problem during configuration or during the build itself.

@UniqMartin
Copy link
Contributor

Yes, sadly no luck there. Thanks for trying and keep the ideas coming if they pop into your head. I'll try to diagnose this further in the coming days, but don't have the time to do that right now.

@Ashish-Bansal
Copy link

Hi, I would like to inform that Qt 5.6.1 link is dead[0] at[1] as it has been replaced by new revision[2]. I know you are trying to fix problems with Qt 5.7 and will update formula for that but I guess for the time being, master should be fixed for not having that problem :)

[0] https://download.qt.io/official_releases/qt/5.6/5.6.1/single/qt-everywhere-opensource-src-5.6.1.tar.xz
[1] https://github.com/Homebrew/homebrew-core/blob/master/Formula/qt5.rb#L17
[2] https://download.qt.io/official_releases/qt/5.6/5.6.1-1/single/

PS: I have just shifted to Mac from my linux machine and Thanks to homebrew for rescuing me!

@ilovezfs ilovezfs mentioned this pull request Jun 23, 2016
4 tasks
@UniqMartin UniqMartin mentioned this pull request Jun 23, 2016
4 tasks
@UniqMartin
Copy link
Contributor

@Ashish-Bansal Thank you for letting us know! I created #2350 to address this, but it will still take a few hours until this has passed CI, bottles have been built, and the PR can be merged. It's rather annoying when upstream decides to completely remove a previously published release from their download servers.

@Ashish-Bansal
Copy link

@UniqMartin Thanks! And yes, I completely agree with that, they shouldn't really delete the previous revision from their servers.

@The-Compiler
Copy link
Contributor

I tried to take a look at this, but I'm a bit clueless as well. I tried asking in the #qtwebengine IRC channel on Freenode, I'll let you know if something comes up.

I also diffed the configure output between 10.9 and 10.11 - other than some failing checks, here's the difference:

--- 01.configure-10.11  2016-06-30 09:29:48.596819069 +0200
+++ 01.configure-10.9   2016-06-30 09:29:48.610152403 +0200
@@ -1,17 +1,17 @@
 Build options:
-  Configuration .......... accessibility audio-backend avx avx2 avx512cd avx512er avx512f c++11 c++14 c++1z compile_examples concurrent corewlan cups doubleconversion freetype full-config getaddrinfo getifaddrs harfbuzz iconv ipv6ifname large-config largefile medium-config minimal-config nis no-pkg-config opengl pcre png poll_poll precompile_header qpa qpa qt_framework reduce_exports release securetransport shared small-config sse2 sse3 sse4_1 sse4_2 ssl ssse3 system-zlib
+  Configuration .......... accessibility audio-backend avx avx2 c++11 c++14 compile_examples concurrent corewlan cups doubleconversion freetype full-config getaddrinfo getifaddrs harfbuzz iconv ipv6ifname large-config largefile medium-config minimal-config nis no-pkg-config opengl pcre png poll_poll precompile_header qpa qpa qt_framework reduce_exports release securetransport shared small-config sse2 sse3 sse4_1 sse4_2 ssl ssse3 system-zlib use_gold_linker
   Build parts ............ libs tools
   Mode ................... release
   Using sanitizer(s)...... none
-  Using C++ standard ..... c++1z
-  Using gold linker....... no
+  Using C++ standard ..... c++14
+  Using gold linker....... yes
   Using new DTAGS ........ no
   Using PCH .............. yes
   Using LTCG ............. no
   Target compiler supports:
     SSE .................. SSE2 SSE3 SSSE3 SSE4.1 SSE4.2
     AVX .................. AVX AVX2
-    AVX512 ............... AVX512F AVX512ER AVX512CD
+    AVX512 ............... <none>

 Qt modules and options:
   Qt D-Bus ............... no

So nothing new to what you already mentioned above...

@The-Compiler
Copy link
Contributor

FWIW, this change mentions it needs SDK 10.10_.3_ or later - I only can see it's using 10.10 from the build log, maybe with an older patch version?

@The-Compiler
Copy link
Contributor

Seems like that's the culprit indeed - IRC logs:

  • The-Compiler: Afaik webengine 5.7 is not going to be officially supported for 10.9 anymore
  • The-Compiler: Also without the build log, it's kind of a guessing game what went wrong
  • The-Compiler: My initial guess would be that if the build is done with the 10.10 sdk shipped with xcode 6.2, then it will not be enough, because it requires a newer 10.10 sdk
  • alcroito: yeah, I just mentioned that guess as well - seems like 10.10.3 is required?
  • The-Compiler: Yes, which ships minimum with xcode 6.3, which is not supported on 10.9, which is why we will probably not support it either
  • alcroito: FWIW http://doc.qt.io/qt-5/qtwebengine-platform-notes.html#os-x says "On OS X, Xcode version 5.1 or later on OS X 10.9 or later is required." for 5.7 which implies that'd work
  • The-Compiler: That is outdated, and we will have to update it. We discovered that 10.9 won't work fairly recently
  • The initial assumption was that 10.10.3 sdk would be shipped with xcode 6.2, but that's not the case.
  • So homebrew would have to drop QtWebEngine for 10.9 with Qt 5.7?
  • You can of course take the 10.10.3 from a newer xcode, and build with the older one, but that might not work forever

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jun 30, 2016

I believe the formula scripts can be fairly complicated. It sounds to me like we may need to have a rule, such that building qt5 without a "no webengine" argument would be unsupported on 10.9. We might be able to get past that by manually replacing the bottle for 10.9 with the 10.10 version, but I don't know how easy it would be to do that...

The other thing we could do is manually update the SDK version. I remember reading something about setting a custom SDK version in Xcode before. It may not be that easy with a build script (instead of an .xcodeproj file), but it might work (and save us the headache later).

@UniqMartin
Copy link
Contributor

FWIW, this change mentions it needs SDK 10.10_.3_ or later - I only can see it's using 10.10 from the build log, maybe with an older patch version?

@The-Compiler Thanks for finding this and for investigating this further! This commit pretty much explains it all. So the bottom line is that the QtWebEngine module cannot be built with an SDK prior to 10.10.3, meaning this automatically excludes OS X 10.9 as the latest officially supported Xcode is 6.2.

The saddening part is that this nontrivial change to the build requirements isn't documented anywhere (as far as I can see), neither in the documentation you pointed out nor the Qt 5.7.0 Change Files.

Building on a newer machine (but with a deployment target of 10.9) and then using this is theoretically possible, but doesn't work with our CI setup. Modifying the Xcode that we use on our 10.9 CI is not an option (and not practical either), as that could create more trouble for other formulae and will only help with the bottles, but 10.9 users building from source (with Xcode 6.2) would still be left out.

I'm currently seeing these options:

  1. Delay shipping Qt 5.7.0 until macOS 10.12 Sierra has been released. At this point we no longer care about OS X 10.9 and stop providing bottles for it anyway. (This is at least practical in the sense that Qt 5.6 is a long-term release and will receive future updates.)
  2. Ship Qt 5.7.0 now, but live with the problem that the 10.9 bottle will provide an inferior experience, namely the missing QtWebEngine module and related modules. (This sounds problematic as it is likely to create confusion for users and thus additional support burden for us. And it impacts all other formulae that depend on Qt 5 and might be depending on QtWebEngine.)
  3. Ship Qt 5.7.0 now, but raise the minimum required OS X version to 10.10. (This provides a more consistent user experience, but will be annoying to those who want other bits of Qt and don't necessarily require the QtWebEngine module. We'd also have to figure out how to deal with this situation in all formulae that depend on Qt 5.)

If I had to order these options by preference, that would be: 1, 3, 2. @DomT4 What are your thoughts on the situation and the options we have here? Any other options that I'm overlooking?

@haraldF
Copy link
Contributor

haraldF commented Jul 14, 2016

ok, loads of information :) Can we get a quick tl;dr?

From what I can see, Qt 5.7 is not in homebrew as QtWebEngine doesn't compile on older OS X versions.

However, there's also a comment indicating that this is fixed in the 5.7 - branch, so it's a matter of backporting a couple of fixes? Or is is generally not supported by the Qt folks any more? (In which case I might be able to help by driving over to the Qt Adlershof office with a crate of beer ;))

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jul 14, 2016

Can we get a quick tl;dr?

@haraldF The "quick tl;dr" is that an ideological war is starting on how these sorts of projects should be distributed.

As to the status of the PR, we're already tried 2 patches from the 5.7 branch, but are still experiencing compile errors. There has been some discussion about just dropping support for the versions of OS X that are failing, but that would involve editing a large number of formulas dependent on it. If you know of what other patches are needed to get this working, link them here, and I'd be willing to update the PR with them and let the bot take another attempt at it (if the project members are willing to let it run for long enough).

@UniqMartin
Copy link
Contributor

Yes, @LRFLEW summarized it nicely. I think the simple/desirable solution is one of:

  • Identify the missing changes between 5.7.0 and the 5.7 branch that hopefully restore compatibility with 10.9 as a build configuration, for reasons explained above.
  • Wait for 5.7.1 that should include all those fixes.

All other solutions require more work (and will probably result in some grief for the 10.9 folks) and also mean someone would need to step up and do this work.

@The-Compiler
Copy link
Contributor

FYI: Looks like that requirement gets bumped to 10.10 again with Qt 5.8 as Chromium doesn't build anymore otherwise.

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Aug 5, 2016

Hmm, if that's true, then we may want to reconsider what option we take. @UniqMartin What's your thoughts?

@The-Compiler
Copy link
Contributor

Though Alexandru Croitor (== @placinta?) apparently did another patch which is in review to get it to build again, so there's hope... What a roller coaster 😉

@UniqMartin
Copy link
Contributor

Since 10.9 is no longer a supported build environment for the QtWebEngine module (inherited from Chromium that it is built on top of) but we want to either ship a complete Qt will all of its modules and we want/have to continue to support from-source builds, I'm increasingly of the opinion that we should just bite the bullet and raise the minimum macOS requirement to 10.10.

Though Alexandru Croitor has so far done a wonderful job of restoring compatibility with older build environments on macOS, this will always be a game of catching up and may very soon turn out to be impractical. And I fear it's always going to be an afterthought in the overall Qt release process, so future breakage seems likely.

If @DomT4 and @MikeMcQuaid don't have strong feelings that we should pursue a different path, let's just raise the minimum macOS requirement and kill 10.9 support in qt5 and all formulae that have a hard dependency on qt5. 10.9 support will soon be phased out anyway as 10.12 is already on the horizon and our <= 10.9 demographic is fairly small at this point.

@alcroito
Copy link

alcroito commented Aug 5, 2016

To be fair, if I do end up fixing 10.9 compilation on Qt 5.8, and it is fixed for 5.7.1, the only catching up will have to be for patch releases of 5.8. And given that the Chromium version will not be updated anymore for the 5.8 patch releases of Qt, there is little chance something will break after the initial hurdle. And in Qt 5.9, 10.9 support will be officially dropped for all Qt modules.

The patch that @The-Compiler mentioned (requiring 10.10) was committed because the 5.7 -> 5.8 merge was blocked by the 10.9 compilation issues. If I fix the 10.9 issues, I'll lower the requirements back to 10.9.

@MikeMcQuaid
Copy link
Member

Since 10.9 is no longer a supported build environment for the QtWebEngine module (inherited from Chromium that it is built on top of) but we want to either ship a complete Qt will all of its modules and we want/have to continue to support from-source builds, I'm increasingly of the opinion that we should just bite the bullet and raise the minimum macOS requirement to 10.10.

And I fear it's always going to be an afterthought in the overall Qt release process, so future breakage seems likely.

I agree 100%. If/when our users complain we can point them to Qt to voice their complaints.

@DomT4
Copy link
Contributor

DomT4 commented Aug 5, 2016

Aye. It might be good to give Travis a heads-up given a decent chunk of our Qt5 usage is CI based and Travis' default CI is 10.9, but in principle I agree we're fighting a losing battle here.

@MikeMcQuaid
Copy link
Member

@DomT4 We probably owe Travis an email anyway to try and convince them to update their images to the split brew/core repos.

@DomT4
Copy link
Contributor

DomT4 commented Aug 5, 2016

Yeah. It'd be helpful to everyone if they upgrade some of their preinstalled formulae as well; at the moment a wad of people using Travis have had to add upgrade checks to libtool & pkg-config after the changes we made internally.

A general update & raising the default OS X image to at least 10.10 would be ideal on all sides, IMO. We can at least reassure them that once Homebrew is 1.0 the breaking changes to core code are going to be less frequent.

@MikeMcQuaid
Copy link
Member

A general update & raising the default OS X image to at least 10.10 would be ideal on all sides, IMO. We can at least reassure them that once Homebrew is 1.0 the breaking changes to core code are going to be less frequent.

Alternatively: we wait until 1.0 and then guarantee it.

lucafavatella added a commit to lucafavatella/homebrew-core that referenced this pull request Aug 11, 2016
The build error would manifest as follows:
```
...
Making install in libparser
clang -DHAVE_CONFIG_H -I. -I..  -I../libparser  -I../libutil -I../libdb -I../libglibc -I../libutil   -g -O2 -c -o parser.o parser.c
clang -DHAVE_CONFIG_H -I. -I..  -I../libparser  -I../libutil -I../libdb -I../libglibc -I../libutil   -g -O2 -c -o C.o C.c
clang -DHAVE_CONFIG_H -I. -I..  -I../libparser  -I../libutil -I../libdb -I../libglibc -I../libutil   -g -O2 -c -o Cpp.o Cpp.c
make[1]: *** No rule to make target `asm_parse.c', needed by `asm_parse.o'.  Stop.
make: *** [install-recursive] Error 1
```

The call `sh reconf.sh` does not fail even if bison fail.  Enabling
`set -x` in the script showed the following:
```
...
+'[' -f asm_parse.y ']'
+bison -d -o asm_parse.c asm_parse.y
asm_parse.y:76.14-19: syntax error, unexpected string, expecting =
...
```

global build deps ref: http://cvs.savannah.gnu.org/viewvc/global/global/libparser/HACKING?revision=1.4&view=markup

gperf OSX refs:
* Homebrew/legacy-homebrew#7794 (comment)
* Homebrew/legacy-homebrew#11039 (comment)
* Homebrew#2087 (comment)
DomT4 pushed a commit that referenced this pull request Aug 12, 2016
The build error would manifest as follows:
```
...
Making install in libparser
clang -DHAVE_CONFIG_H -I. -I..  -I../libparser  -I../libutil -I../libdb -I../libglibc -I../libutil   -g -O2 -c -o parser.o parser.c
clang -DHAVE_CONFIG_H -I. -I..  -I../libparser  -I../libutil -I../libdb -I../libglibc -I../libutil   -g -O2 -c -o C.o C.c
clang -DHAVE_CONFIG_H -I. -I..  -I../libparser  -I../libutil -I../libdb -I../libglibc -I../libutil   -g -O2 -c -o Cpp.o Cpp.c
make[1]: *** No rule to make target `asm_parse.c', needed by `asm_parse.o'.  Stop.
make: *** [install-recursive] Error 1
```

The call `sh reconf.sh` does not fail even if bison fail.  Enabling
`set -x` in the script showed the following:
```
...
+'[' -f asm_parse.y ']'
+bison -d -o asm_parse.c asm_parse.y
asm_parse.y:76.14-19: syntax error, unexpected string, expecting =
...
```

global build deps ref: http://cvs.savannah.gnu.org/viewvc/global/global/libparser/HACKING?revision=1.4&view=markup

gperf OSX refs:
* Homebrew/legacy-homebrew#7794 (comment)
* Homebrew/legacy-homebrew#11039 (comment)
* #2087 (comment)
@MikeMcQuaid
Copy link
Member

@LRFLEW Any news on this? Thanks!

@DomT4 DomT4 removed their assignment Sep 1, 2016
@MikeMcQuaid MikeMcQuaid closed this Sep 3, 2016
@BrewTestBot BrewTestBot removed the in progress Stale bot should stay away label Sep 3, 2016
@MikeMcQuaid
Copy link
Member

Will wait for Qt 5.7.1.

@mistydemeo mistydemeo mentioned this pull request Sep 20, 2016
@schoeps
Copy link
Contributor

schoeps commented Sep 21, 2016

... or ship a snapshot?

@schoeps
Copy link
Contributor

schoeps commented Oct 3, 2016

qt5-qtconnectivity seems to be fixed https://trac.macports.org/ticket/52203

@MikeMcQuaid
Copy link
Member

If we can get something that compiles on our now three supported platforms (10.10 - 10.12) I'm happy to include it.

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.