Bug 126324

Summary:

configure check for clang version is incorrect

Product:

WebKit

Reporter:

Jeremy Huddleston Sequoia <jeremyhu>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, berto, cgarcia, commit-queue, mcatanzaro, ossy, philip.chimento, tpopela, zan

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

126492

Attachments:

Description	Flags
trivial patch	none
clang-check.patch	none
Updated patch	none
Patch	cgarcia: review+, cgarcia: commit-queue-

Jeremy Huddleston Sequoia

Reported 2013-12-30 21:13:05 PST

Created attachment 220144 [details] trivial patch The logic to verify >= clang-3.2 is incorrect: 1) It does not correctly check Apple clang versions 2) It forces minor version >= 2 even when major version > 3 (eg 4.0 will fail) Trivial patch attached

Attachments
trivial patch (1.10 KB, patch) 2013-12-30 21:13 PST, Jeremy Huddleston Sequoia	no flags	Details Formatted Diff Diff
clang-check.patch (1.54 KB, patch) 2014-02-05 17:32 PST, Jeremy Huddleston Sequoia	no flags	Details Formatted Diff Diff
Updated patch (2.02 KB, patch) 2014-10-03 01:09 PDT, Philip Chimento	no flags	Details Formatted Diff Diff
Patch (2.56 KB, patch) 2014-10-05 20:17 PDT, Philip Chimento	cgarcia: review+ cgarcia: commit-queue-	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Csaba Osztrogonác

Comment 1 2014-01-13 06:01:51 PST

Please add changelog and set r? if you would like to ask for review.

Jeremy Huddleston Sequoia

Comment 2 2014-02-05 17:32:48 PST

Created attachment 223284 [details] clang-check.patch

WebKit Commit Bot

Comment 3 2014-02-05 17:34:53 PST

Attachment 223284 [details] did not pass style-queue: Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.

Philip Chimento

Comment 4 2014-10-03 01:09:59 PDT

Created attachment 239190 [details] Updated patch The same logic error was made with GCC versions too. Here's an updated patch.

Michael Catanzaro

Comment 5 2014-10-03 07:22:47 PDT

Note that the autotools build was removed for 2.6. The version check is indeed incorrect and will break when GCC 5 is released next spring, but the proposed fix is wrong too. The right hand side of the || will always be short-circuited: (__GNUC__ >= 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) You want: (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) The altered Clang version check is correct.

Philip Chimento

Comment 6 2014-10-05 20:17:18 PDT

Created attachment 239312 [details] Patch

Philip Chimento

Comment 7 2014-10-05 20:19:14 PDT

Thanks for catching that. Here's another one. I know the autotools build is obsolete but it would be nice if this could be committed to the 2.4 branch.

Philip Chimento

Comment 8 2014-11-05 22:06:03 PST

Would it be possible to get this reviewed for 2.4.8?

Philip Chimento

Comment 9 2015-01-18 12:17:11 PST

Would it be possible to review this for 2.4.9?

Michael Catanzaro

Comment 10 2015-02-10 18:47:04 PST

Carlos, if you're planning to do another 2.4 release, can you take this patch please? We're going to use this patch in Fedora now that we've upgraded to the GCC 5.0 prerelease.

Carlos Garcia Campos

Comment 11 2015-02-11 00:19:15 PST

Yes, I will. I already had this in my todo list for the next 2.4, but please, remember that the best way to make sure I don't forget a bug/patch is adding it to the list of proposed merges in http://trac.webkit.org/wiki/WebKitGTK/2.4.x

Philip Chimento

Comment 12 2015-02-11 22:19:25 PST

Sure thing. I somehow thought that that page was only for reviewed patches, but I'll add them there in the future.

Carlos Garcia Campos

Comment 13 2015-02-12 00:07:37 PST

(In reply to comment #12) > Sure thing. I somehow thought that that page was only for reviewed patches, > but I'll add them there in the future. Not necessarily, the rule might be something like this: - If it has landed, add it as proposed merge for next release - If it's important bug, add it as proposed merge for next release - Otherwise, or in doubt, add it as changes on track - When adding a landed patch, please use the trac changeset instead of the bug url Note that I'll check every change before merging, so it's better to add something there even if you are not sure, that not adding it. But, please, make sure the change hasn't been merged yet (not all changes merged are in the wiki)

Alberto Garcia

Comment 14 2015-02-12 08:21:38 PST

I confirm that 2.4.8 builds fine with gcc 5 and these changes.

Carlos Garcia Campos

Comment 15 2015-04-07 02:29:50 PDT

Committed <http://trac.webkit.org/changeset/182457>

Note You need to log in before you can comment on or make changes to this bug.