Bug 41953 - build fails - MathExtras.h on Solaris
Summary: build fails - MathExtras.h on Solaris
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P3 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-07-09 04:57 PDT by Pavel Heimlich (hajma)
Modified: 2013-01-02 07:32 PST (History)
7 users (show)

See Also:


Attachments
patch (571 bytes, patch)
2010-07-09 04:57 PDT, Pavel Heimlich (hajma)
no flags Details | Formatted Diff | Diff
Patch to fix compile issue on Solaris 10/Sun Studio 12 CC in JavaScriptCore/wtf/MathExtras.h (1.23 KB, patch)
2011-03-28 20:15 PDT, Ben Taylor
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Heimlich (hajma) 2010-07-09 04:57:32 PDT
Created attachment 61034 [details]
patch

building qt 4.7 beta2 fails with
...
"./wtf/MathExtras.h", line 194: Error: isfinite is not a member of std.
"./wtf/MathExtras.h", line 195: Error: isinf is not a member of std.
"./wtf/MathExtras.h", line 196: Error: isnan is not a member of std.
"./wtf/MathExtras.h", line 197: Error: signbit is not a member of std.

the attached patch fixes that by adding Solaris to the ifdef already provided for MSVC and Android


this is on OpenSolaris b134 using the Sun Studio 12u1 compiler
Comment 1 t.hirsch@web.de 2011-01-04 14:36:03 PST
I can confirm this bug.

Please apply Pavel's patch! It's working fine.
Comment 2 t.hirsch@web.de 2011-01-04 14:42:30 PST
Actually I think that the code in trunk should also work:

#if !COMPILER(MSVC) && !COMPILER(WINSCW) && !(COMPILER(RVCT) && (OS(SYMBIAN) || PLATFORM(BREWMP)))

Here's how it was before and what is still being used in Qt 4.7:

#if !COMPILER(MSVC) && !COMPILER(RVCT) && !OS(ANDROID) && !COMPILER(WINSCW)

I'll tell the Nokia guys to update their webkit...


Looks like this bug can be closed. Pavel, do you agree?
Comment 3 Pavel Heimlich (hajma) 2011-01-04 15:05:04 PST
(In reply to comment #2)

> Looks like this bug can be closed. Pavel, do you agree?

it certainly does not look like anybody from webkit cares about buildability on solaris, so for me it does not make any sense to spend time on development versions. I'll tell when the next Qt version is out.
Comment 4 Alexey Proskuryakov 2011-01-04 17:25:34 PST
For the record, we gladly accept build fixes, but please don't disregard <http://webkit.org/coding/contributing.html>.
Comment 5 Alexey Proskuryakov 2011-01-04 17:26:47 PST
In particular, patches that are not marked r? likely won't be noticed, and patches without ChangeLogs will be not accepted.
Comment 6 Benjamin Poulain 2011-01-07 03:16:24 PST
(In reply to comment #3)
> it certainly does not look like anybody from webkit cares about buildability on solaris, so for me it does not make any sense to spend time on development versions. I'll tell when the next Qt version is out.

We don't have a buildbot for solaris but it is certainly supported. We accept patches for sure.

The problem here is that nobody saw you patch until now. I just learned about it because a colleague triaged this http://bugreports.qt.nokia.com/browse/QTBUG-16448

There are two problems in this bug report that made it invisible for us:

First, you have not used the template to report this bug. Since the keyword "Qt" is missing, this bug was never triaged by us. See http://trac.webkit.org/wiki/QtWebKitBugs for more info.

Second, your patch was missing the changelog and was not marked for review. Such a patch does not appear in the review list. See http://trac.webkit.org/wiki/QtWebKitContrib for more info.

Finally, if a patch is ignored, you can always write on the WebKit mailing list (webkit-qt and webkit). We welcome contribution, you were just unlucky and nobody from Qt saw the bug.


(In reply to comment #2)
> Looks like this bug can be closed. Pavel, do you agree?

Closing the bug then.
Comment 7 Ben Taylor 2011-03-28 18:34:57 PDT
Reopen this bug.  The #if line from comment #2 fails to compile on Solaris 10 with Sun Studio 12, and add "&& !OS(SOLARIS)" fixes the problem.

I will submit a new patch
Comment 8 Ben Taylor 2011-03-28 20:15:59 PDT
Created attachment 87253 [details]
Patch to fix compile issue on Solaris 10/Sun Studio 12 CC in JavaScriptCore/wtf/MathExtras.h

The comment in #2 surely doesn't fix the issue on Solaris 10/Sun Studio 12 CC, as compiled with the qt-4.7.2.
Adding the patch completed the compile.
Comment 9 Benjamin Poulain 2011-03-29 05:45:08 PDT
(In reply to comment #7)
> Reopen this bug.  The #if line from comment #2 fails to compile on Solaris 10 with Sun Studio 12, and add "&& !OS(SOLARIS)" fixes the problem.
> 
> I will submit a new patch
Comment 10 Benjamin Poulain 2011-03-29 05:53:36 PDT
Pierre does that make sense to you?

The change looks good to me but I would like a confirmation that the STL is always broken on Solaris. What about the GNU tools on Solaris?
Comment 11 Pierre Rossi 2011-03-29 06:15:15 PDT
(In reply to comment #10)
> Pierre does that make sense to you?
> 
> The change looks good to me but I would like a confirmation that the STL is always broken on Solaris. What about the GNU tools on Solaris?

Yup, that looks good to me to, my general rule of thumb when it comes to dealing with Solaris is to really lower your expectations, that's what this patch does, so definitely thumbs up ! :)
Comment 12 Benjamin Poulain 2011-03-29 06:20:02 PDT
Comment on attachment 87253 [details]
Patch to fix compile issue on Solaris 10/Sun Studio 12 CC in JavaScriptCore/wtf/MathExtras.h

Look sane and Pierre says ok.
Comment 13 WebKit Commit Bot 2011-03-29 08:26:12 PDT
Comment on attachment 87253 [details]
Patch to fix compile issue on Solaris 10/Sun Studio 12 CC in JavaScriptCore/wtf/MathExtras.h

Clearing flags on attachment: 87253

Committed r82254: <http://trac.webkit.org/changeset/82254>
Comment 14 WebKit Commit Bot 2011-03-29 08:26:18 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 richard 2013-01-02 04:39:45 PST
I believe this latest patch is inappropriate for solaris if GCC is being used.

Perhaps it could better be done by changing:

&& !OS(SOLARIS) 

to 

&& !( OS(SOLARIS) && defined(__SUNPRO_CC) )

or something of this nature.
Comment 16 Eric Seidel (no email) 2013-01-02 07:32:34 PST
Please open a new patch and upload a new bug.  We have very very few solaris users, so if you don't fix it, no one will. :)