Skip to content

qt: remove docs option#29469

Merged
ilovezfs merged 1 commit intoHomebrew:masterfrom
ilovezfs:qt-fix-docs
Jun 27, 2018
Merged

qt: remove docs option#29469
ilovezfs merged 1 commit intoHomebrew:masterfrom
ilovezfs:qt-fix-docs

Conversation

@ilovezfs
Copy link
Copy Markdown
Contributor

add llvm optional dependency
deprecate --with-docs in favor of --with-llvm as qdoc needs libclang

  • 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>)?

Fixes #29415.

@ilovezfs
Copy link
Copy Markdown
Contributor Author

It may make more sense to remove the option altogether.

@apjanke
Copy link
Copy Markdown
Contributor

apjanke commented Jun 27, 2018

Build failure on my 10.13.5 box. :(

Full logs: https://gist.github.com/93c83998c4f522f67696b11db2dc85e0

[/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula]
$ hub checkout https://github.com/Homebrew/homebrew-core/pull/29469                                                                                                           master
remote: Counting objects: 4, done.
remote: Total 4 (delta 3), reused 4 (delta 3), pack-reused 0
Unpacking objects: 100% (4/4), done.
From https://github.com/Homebrew/homebrew-core
 * [new ref]               refs/pull/29469/head -> qt-fix-docs
Switched to branch 'qt-fix-docs'
[...]
$ brew install -s qt --with-llvm                                                                                                                                         qt-fix-docs
==> Installing dependencies for qt: llvm
==> Installing qt dependency: llvm
==> Downloading https://homebrew.bintray.com/bottles/llvm-6.0.0.high_sierra.bottle.tar.gz
######################################################################## 100.0%
==> Pouring llvm-6.0.0.high_sierra.bottle.tar.gz
[...]
==> Installing qt --with-llvm
==> Downloading https://download.qt.io/official_releases/qt/5.11/5.11.1/single/qt-everywhere-src-5.11.1.tar.xz
[...]
==> make docs
Last 15 lines from /Users/janke/Library/Logs/Homebrew/qt/04.make:
cd tools/qfloat16-tables/ && ( test -e Makefile || /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/bin/qmake -o Makefile /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/tools/qfloat16-tables/qfloat16-tables.pro ) && /Applications/Xcode-9.4.app/Contents/Developer/usr/bin/make -f Makefile prepare_docs
make[5]: Nothing to be done for `prepare_docs'.
cd 3rdparty/pcre2/ && ( test -e Makefile || /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/bin/qmake -o Makefile /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/3rdparty/pcre2/pcre2.pro ) && /Applications/Xcode-9.4.app/Contents/Developer/usr/bin/make -f Makefile prepare_docs
make[5]: Nothing to be done for `prepare_docs'.
cd corelib/ && ( test -e Makefile || /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/bin/qmake -o Makefile /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/corelib/corelib.pro ) && /Applications/Xcode-9.4.app/Contents/Developer/usr/bin/make -f Makefile prepare_docs
/usr/local/Cellar/qt/5.11.1/bin/qtattributionsscanner /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase --filter QDocModule=qtcore -o /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/corelib/codeattributions.qdoc
/private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/corelib/qdoc_wrapper.sh -outputdir /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/doc/qtcore -installdir /usr/local/Cellar/qt/5.11.1/doc /private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/corelib/doc/qtcore.qdocconf -prepare -no-link-errors -I. -Iglobal -I../3rdparty/harfbuzz/src -I../3rdparty/md5 -I../3rdparty/md4 -I../3rdparty/sha3 -I../3rdparty/double-conversion/include -I../3rdparty/double-conversion/include/double-conversion -I../3rdparty/forkfd -I../../include -I../../include/QtCore -I../../include/QtCore/5.11.1 -I../../include/QtCore/5.11.1/QtCore -I.moc -I.tracegen -I../3rdparty/pcre2/src -I../../mkspecs/macx-clang
/private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/corelib/qdoc_wrapper.sh: line 12: /usr/local/Cellar/qt/5.11.1/bin/qdoc: No such file or directory
/private/tmp/qt-20180627-99636-1enfl8q/qt-everywhere-src-5.11.1/qtbase/src/corelib/qdoc_wrapper.sh: line 12: exec: /usr/local/Cellar/qt/5.11.1/bin/qdoc: cannot execute: No such file or directory
make[5]: *** [prepare_docs] Error 126
make[4]: *** [sub-corelib-prepare_docs] Error 2
make[3]: *** [sub-src-prepare_docs] Error 2
make[2]: *** [module-qtbase-prepare_docs] Error 2
make[1]: *** [html_docs] Error 2
make: *** [docs] Error 2

@ilovezfs
Copy link
Copy Markdown
Contributor Author

OK, we'll remove the option then.

@ilovezfs ilovezfs changed the title qt: fix building documentation qt: remove docs option Jun 27, 2018
@ilovezfs ilovezfs merged commit 571b462 into Homebrew:master Jun 27, 2018
@RandomDSdevel
Copy link
Copy Markdown
Contributor

RandomDSdevel commented Jun 27, 2018

@apjanke:

     Huh, there's that same error again. Now that I consider one more time, though, I remember wondering why Qt's build infrastructure might consider it a good idea to look for qdocand other build tools where Homebrew would install them in the cellar along with all of the rest of Qt instead of in Homebrew's temporary Qt build directory…before Homebrew had built and atomically installed Qt in its entirety into the cellar. (My apologies for not sharing this earlier, but it was only a fleeting insight and only bubbled back up to the front of my mind just now.) (Never mind, see addendum below.)

Addendum 1:

     🤦‍♂️ Whoops, just realized a little while ago that Qt's build process might start putting its documentation together after installing most everything else into the Cellar. The atomic part would be Homebrew linking things from the Cellar into Qt's opt_prefix.

Addendum 2:

     And I also just realized that this is what @mistydemeo meant in #29415 (comment). It appears I'm a bit slow lately…😅.


@ilovezfs:

OK, we'll remove the option then.

     See #29415 (comment), parts 1 and 4.

@RandomDSdevel
Copy link
Copy Markdown
Contributor

RandomDSdevel commented Jul 2, 2018

@apjanke, @ilovezfs:

     I may revisit this. A reason this attempt might have failed could be that we need to pull the upstream patch that @msorvig mentioned in #29415 down in order to make Qt's build system look for libclang in Homebrew's LLVM instead of inside Xcode's. (I know Homebrew prefers using system tools when they're available, but apparently Qt upstream may not want to depend on a library private to Xcode's internals, and I daresay honoring that decision would be preferred somewhat heavily to bending ourselves over backwards trying to work around this change and put things back to how they were before, roughly speaking. I've asked about whether this is really the line of reasoning they might be following on their end, so we'll see what they say…)

@ilovezfs
Copy link
Copy Markdown
Contributor Author

ilovezfs commented Jul 2, 2018

@RandomDSdevel they're hard coding the Homebrew prefix as /usr/local so that patch is sketchy. I assume it has some sort of fall back to just use llvm-config to find things, which should work fine. I have no idea why the build failed, though, if that's the case.

@RandomDSdevel
Copy link
Copy Markdown
Contributor

RandomDSdevel commented Jul 2, 2018

@ilovezfs:

     Its commit message says you can still configure where Qt's build system looks for LLVM by setting the LLVM_INSTALL_DIR environment variable, so, yes. I might try a brew install --interactive later to see if I can't debug this, then.

@ilovezfs
Copy link
Copy Markdown
Contributor Author

ilovezfs commented Jul 2, 2018

@RandomDSdevel ah OK. I have no idea why they'd require us to set that if llvm-config is in the PATH but yes that may work.

@ilovezfs
Copy link
Copy Markdown
Contributor Author

ilovezfs commented Jul 2, 2018

In particular,

iMac-TMP:~ joe$ /usr/local/opt/llvm/bin/llvm-config --prefix
/usr/local/Cellar/llvm/6.0.0

So why should we need to set LLVM_INSTALL_DIR? hehe

@RandomDSdevel
Copy link
Copy Markdown
Contributor

RandomDSdevel commented Jul 2, 2018

     I agree it does seem a bit obtuse, to say the least… (Addendum: Perhaps we're missing something here that's interfering with Qt's default handling of this…)

@RandomDSdevel
Copy link
Copy Markdown
Contributor

RandomDSdevel commented Jul 2, 2018

@ilovezfs:

     I've gone ahead and relayed your feedback in #29469 (comment) on upstream hardcoding Homebrew's prefix to them in a new comment on the relevant change if that's all right with you. (The comment editor messed up my quote of brew's man page's section on the command's --prefix option, though; it mangled my line breaking…🤷‍♂️.)

@RandomDSdevel
Copy link
Copy Markdown
Contributor

@ilovezfs: Is there any equivalent of brew install --interactive for brew upgrade short of manually making a temporary backup of a package's installation state?

@RandomDSdevel
Copy link
Copy Markdown
Contributor

@ilovezfs:

Me earlier:

…I might try a brew install --interactive later to see if I can't debug this, then.

     With regards to that, I'm still working on seeing if there isn't anything obvious in either how Homebrew currently builds Qt or how having it pull in one or more of the patches mentioned in this and/or related discussion conversation might possibly affect things. It's slow going, though, given how much make output I have to sift through after the fact and that nothing's really jumped out at me yet. (Practically freezing Terminal and my system in general by jamming my scrollback buffer chock full with untold scads of make debugging information that I thought might help but actually just buried any useful output didn't help, either, but I digress…)

(CCing @apjanke since he's also been involved here and with #29415 and might want a progress update, underwhelming as it is…)

@RandomDSdevel
Copy link
Copy Markdown
Contributor

RandomDSdevel commented Jul 20, 2018

@ilovezfs, @apjanke:

     I've been having a little more trouble than anticipated reproducing Qt's build system's failure to resolve LLVM, particularly libclang, inside brew install -vd --build-from-source --interactive qt (as opposed to a non-interactive install --with-docs,) as:

  • going through the latter appears to invoke make with its -S/--no-keep-going/--stop option somehow (maybe adding it somewhere, perhaps in Debrew, that I missed when I recently went and did a bit of spelunking about brew's codebase to make sure I wasn't omitting any configure and make options that Homebrew might add when triggered to be --verbose and output --debugging information…? 🤔🤷‍♂️.)
  • wading through the former appears to be exhibiting the behavior one would manually trigger using make's -k/--keep-going option (and, additionally, possibly its -i/--ignore-errors option as well.)

In any case, I'll continue to investigate and see if I can't figure out where exactly this keels over prior to applying upstream's patches just to make absolutely sure those are definitively targeted as solutions to the problem we actually suspect (albeit rather heavily) we've been facing (since we've only really ever seen a symptom of this in the form of witnessing that Qt's documentation won't build after the fact because qdoc never got built.) At this point, however, it may simply come down to just blindly applying said changes by amending Qt's formula to include the requisite patch do blocks, crossing fingers on both hands after crossing both arms behind my back, and attempting a fresh non-interactive build. Do let me know if this sounds like overkill, though, as I've been known to dabble in that from time to time…😆😅.

P. S.:

     Part of the difficulty may lie in that I keep missing steps from a non-interactive build and installation that one has to follow in an interactive one. I keep forgetting that the usual Unix-style routine of ./configure; make; make install documented upstream for Qt isn't entirely sufficient for Homebrew installations of it (due to our pkg-config patch and our having to deal with some .framework difficulties) until I glance back at its formula, funnily enough…

Edit:

     Sorry, never mind; adding --no-keep-going to my make invocations didn't change anything, at least that I could see. I suppose I can now presume that this behavior is probably make's default, but using it neither implicitly nor explicitly, per my next post in this thread, revealed anything else useful.

@RandomDSdevel
Copy link
Copy Markdown
Contributor

@ilovezfs, @apjanke:

     I've disappointingly and unfortunately been unable to recover any additional debugging information from any further Qt build attempts intended, as discussed earlier in this thread, to gather such data. I'll put together a PR to bring back Qt's documentation using those patches from upstream once @albertosottile finishes up working on #30229 since I'll of course need that, too. In the meantime, it may be a good idea to continue by moving discussion of further efforts back to #29415 where this all began, as that's the issue that any PR made to fix it would address (perhaps it should be opened again…?)

@albertosottile
Copy link
Copy Markdown
Contributor

@RandomDSdevel, @ilovezfs

once @albertosottile finishes up working on #30229

As far as I am concerned, the PR is ready for review.

@RandomDSdevel
Copy link
Copy Markdown
Contributor

RandomDSdevel commented Jul 23, 2018

@albertosottile:

As far as I am concerned, the PR is ready for review.

     Whoops, I meant 'and/with @ilovezfs,' too, as I was really trying to keep track of #30229's review/merge status. Sorry about that…

@lock lock bot added the outdated PR was locked due to age label Aug 22, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants