| 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: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 126492 | ||||||||||||
| Attachments: |
|
||||||||||||
Please add changelog and set r? if you would like to ask for review. Created attachment 223284 [details]
clang-check.patch
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.
Created attachment 239190 [details]
Updated patch
The same logic error was made with GCC versions too. Here's an updated patch.
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. Created attachment 239312 [details]
Patch
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. Would it be possible to get this reviewed for 2.4.8? Would it be possible to review this for 2.4.9? 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. 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 Sure thing. I somehow thought that that page was only for reviewed patches, but I'll add them there in the future. (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) I confirm that 2.4.8 builds fine with gcc 5 and these changes. Committed <http://trac.webkit.org/changeset/182457> |
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