[webkit-reviews] review denied: [Bug 81750] [Qt][Mac] ranlib segfaults when creating symbol tables for libWebCore.a. : [Attachment 133018] patch for review.

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has denied Zeno Albisser
<zeno at webkit.org>'s request for review:
Bug 81750: [Qt][Mac] ranlib segfaults when creating symbol tables for
libWebCore.a.
https://bugs.webkit.org/show_bug.cgi?id=81750

Attachment 133018: patch for review.
https://bugs.webkit.org/attachment.cgi?id=133018&action=review

------- Additional Comments from Tor Arne Vestbø <vestbo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=133018&action=review


> Source/WebCore/Target.pri:3487
> +	   contains(CONFIG, USE_SVG_ALL_IN_ONE) {

If you do use CONFIG then you can use the CONFIG(foo) macro, or even just the
foo:bar: syntax.

> Tools/qmake/mkspecs/features/default_pre.prf:127
> +macx {
> +    contains(CONFIG, debug)|contains(CONFIG, debug_and_release) {
> +	   message("Building WebKit Debug on mac. Switching to
USE_SVG_ALL_IN_ONE configuration.")
> +	   CONFIG += USE_SVG_ALL_IN_ONE
> +    }
> +}

You can move this part into features/macx/default_pre.prf, which is specific to
mac.  (after loading default_pre).

The message is fine, but prefix it with: root_project_file:message()..
otherwise you'll see it for every single project file

You should not have to check for debug_and_release, CONFIG will already contain
debug i that case.

For the CONFIG option, please lower-case and generalize it, eg,
use_all_in_one_files, that way the config will still apply if we need to add
another all-in-one file