Last Comment Bug 659851 - [10.7 SDK] [32-bit] Firefox and Lion disagree on GLintptr
: [10.7 SDK] [32-bit] Firefox and Lion disagree on GLintptr
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Steven Michaud [:smichaud] (Retired)
:
Mentors:
Depends on:
Blocks: 661638
  Show dependency treegraph
 
Reported: 2011-05-25 20:11 PDT by Jesse Ruderman
Modified: 2011-07-02 01:52 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
mozconfig (normal 32-bit settings except for 10.6 target) (511 bytes, text/plain)
2011-05-25 20:11 PDT, Jesse Ruderman
no flags Details
Possible fix (1.26 KB, patch)
2011-06-02 14:06 PDT, Steven Michaud [:smichaud] (Retired)
jacob.benoit.1: review+
bas: feedback+
Details | Diff | Review
Copy of patch for someone else to land (1.45 KB, patch)
2011-06-23 07:22 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Review

Description Jesse Ruderman 2011-05-25 20:11:03 PDT
Created attachment 535259 [details]
mozconfig (normal 32-bit settings except for 10.6 target)

I get this error when I try to build a 32-bit Firefox on Lion (Mac OS X 10.7).

> In file included from /System/Library/Frameworks/OpenGL.framework/Headers/OpenGL.h:10,
>                  from /Users/jruderman/mozilla-central/gfx/thebes/GLContextProviderCGL.mm:40:
> /System/Library/Frameworks/OpenGL.framework/Headers/gltypes.h:70: error: conflicting declaration ‘typedef long int GLintptr’
> /Users/jruderman/mozilla-central/gfx/thebes/GLDefs.h:68: error: ‘GLintptr’ has a previous declaration as ‘typedef ptrdiff_t GLintptr’
> /System/Library/Frameworks/OpenGL.framework/Headers/gltypes.h:71: error: conflicting declaration ‘typedef long int GLsizeiptr’
> /Users/jruderman/mozilla-central/gfx/thebes/GLDefs.h:67: error: ‘GLsizeiptr’ has a previous declaration as ‘typedef ptrdiff_t GLsizeiptr’

See also bug 594418, a similar problem on Meego.
Comment 1 Steven Michaud [:smichaud] (Retired) 2011-05-25 21:42:57 PDT
Interesting.

I did a successful 32-bit build on 10.7 about a week ago (after having worked around bug 655339 and before I landed my patch for bug 621117).  So this may be a recent problem.
Comment 2 Steven Michaud [:smichaud] (Retired) 2011-05-26 10:17:21 PDT
I'm not able to reproduce this bug.

I built the current trunk (from code pulled this morning) on the
latest DP of OS X 10.7 (build 11A459e), with the latest XCode 4.1 also
installed (DP 5).

Before I built I had to work around bug 655339 (by following the steps
at bug 655339 comment #21) and bug 659881 (by adding "ac_add_options
--enable-macos-target=10.7" to my mozconfig file).

Did you install some version of MacPorts?  If so which one?

I built all the build prerequisites by hand (and didn't use MacPorts)
-- which might have made a difference.

For reference, here's the mozconfig I used:

export CFLAGS="-g -gfull"
export CXXFLAGS="-g -gfull"
. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-firefox-32bit
mk_add_options MOZ_MAKE_FLAGS=-j4
mk_add_options AUTOCONF=autoconf213
ac_add_options --disable-optimize
ac_add_options --enable-tests
ac_add_options --enable-cpp-rtti
ac_add_options --enable-logrefcnt
ac_add_options --disable-strip
ac_add_options --disable-install-strip
ac_add_options --with-macos-sdk=/Developer/SDKs/MacOSX10.6.sdk

CC="gcc-4.2 -arch i386"
CXX="g++-4.2 -arch i386"
ac_add_options --target=i386-apple-darwin8.0.0
ac_add_options --enable-macos-target=10.7
# bug 491774. crashreporter won't build in cross compile
ac_add_options --disable-crashreporter

HOST_CC="gcc-4.2"
HOST_CXX="g++-4.2"
RANLIB=ranlib
AR=ar
AS=$CC
LD=ld
STRIP="strip -x -S"
CROSS_COMPILE=1
Comment 3 Jesse Ruderman 2011-05-26 17:49:17 PDT
I have some half-busted version of MacPorts, and I set my --enable-macos-target to 10.6 rather than 10.7 to work around the same problem.
Comment 4 Jesse Ruderman 2011-05-26 18:41:26 PDT
Hmm, I didn't realize --with-macos-sdk and --enable-macos-target were different things.

I bet this is another problem that only happens with the 10.7 SDK (you're using the 10.6 SDK).
Comment 5 Steven Michaud [:smichaud] (Retired) 2011-05-26 19:17:28 PDT
> Hmm, I didn't realize --with-macos-sdk and --enable-macos-target
> were different things.

--enable-macos-target sets the value of the MACOSX_DEPLOYMENT_TARGET
environment variable ... though I'm not entirely sure what *that*
does.

--with-macos-sdk is much more straightforward:

Each "SDK" is stored in /Developer/SDKs/, and contains system files
for that version of OS X.  So MacOSX10.5.sdk contains system files
(frameworks, libraries and header files) identical to those found on
OS X 10.5.X, and MacOSX10.6.sdk contains system files identical to
those found on OS X 10.6.X.

Setting --with-macos-sdk makes the build use the header files, and
link against the frameworks/libraries, from the specified version of
OS X.  This should (and in my experience always does) make what you're
building usable on versions of OS X that are the same version as the
SDK or higher.

Setting MACOSX_DEPLOYMENT_TARGET is supposed to accomplish the same
thing.  But in my experience it's not nearly as reliable.

> I bet this is another problem that only happens with the 10.7 SDK.

That's correct.
Comment 6 Jonathan Kew (:jfkthame) 2011-05-27 00:07:58 PDT
(In reply to comment #5)
> Setting MACOSX_DEPLOYMENT_TARGET is supposed to accomplish the same
> thing.

No, it's purpose is different.

--with-macos-sdk chooses an SDK version, as you say; this determines the latest APIs that will be available (as newly-introduced 10.7 functions won't exist in the 10.6 SDK, for example). So if you want to access recent API additions, you need to use the corresponding SDK. The resulting build will work on that OS version or later.

MACOSX_DEPLOYMENT_TARGET is used to tell the toolchain that you want your code to work on versions of the OS *earlier* than the SDK you're using. It will cause any newer APIs you use to be weak-linked, so that the app can still launch without symbol resolution failures on older systems, but then it's your responsibility to check function availability at runtime before trying to call such APIs.
Comment 7 Steven Michaud [:smichaud] (Retired) 2011-05-27 07:19:41 PDT
I stand corrected.

Apple doesn't have much documentation on MACOSX_DEPLOYMENT_TARGET that
I can find, but here are a couple of links:

http://developer.apple.com/library/mac/#technotes/tn2064/_index.html
http://developer.apple.com/library/mac/#releasenotes/Darwin/SymbolVariantsRelNotes/_index.html
Comment 8 Steven Michaud [:smichaud] (Retired) 2011-06-02 14:06:43 PDT
Created attachment 536970 [details] [diff] [review]
Possible fix

OS X 10.7's OpenGL framework (also included in the OS X 10.7 SDK)
includes a new header file (gltypes.h) that doesn't appear in the
OpenGL framework on OS X 10.6 and 10.5.

Both gltypes.h and gl.h define some values that are also defined in
gfx/thebes/GLDefs.h.  GLDefs.h includes code to stop these values from
being multiply defined if gl.h has already been included.  My patch
adds code to keep track of whether or not gltypes.h has been included.
Comment 9 Steven Michaud [:smichaud] (Retired) 2011-06-02 14:08:02 PDT
Comment on attachment 536970 [details] [diff] [review]
Possible fix

I haven't yet built with my patch on other platforms than OS X 10.7.  I'm using the tryserver to do that now.
Comment 10 Steven Michaud [:smichaud] (Retired) 2011-06-03 08:05:33 PDT
> I haven't yet built with my patch on other platforms than OS X 10.7.
> I'm using the tryserver to do that now.

These builds have now finished.  There were no non-spurious failures.
Comment 11 Oleg Romashin (:romaxa) 2011-06-05 09:27:19 PDT
Comment on attachment 536970 [details] [diff] [review]
Possible fix

On, Meego 1.2 sdk it does not help, because glTypes definition included directly into __gl2_h_ ...
Comment 12 Steven Michaud [:smichaud] (Retired) 2011-06-06 14:44:13 PDT
(In reply to comment #11)

So you also see this bug with the Meego 1.2 SDK?

Did it start with version 1.2?  What's the version of gcc (and/or
clang) included with the 1.2 SDK?  What's the version of OpenGL in the
1.2 SDK?  What's the last "working" version of the Meego SDK, and what
are the versions of its gcc (and/or clang) and OpenGL?

The version of the OpenGL framework on OS X 10.5.8 is 1.5.10.  That on
OS X 10.6.7 is 1.6.12.  That on OS X 10.7 (the latest DP, build
11A459e) is 1.7.4.

Oh, and have any bugs been opened on the problems building with the
Meego 1.2 SDK?
Comment 13 Jonathan Kew (:jfkthame) 2011-06-06 14:58:12 PDT
Comment on attachment 536970 [details] [diff] [review]
Possible fix

I don't have a Lion system on hand to look into this further, but it seems wrong for us to be #define'ing __gl_h_ and __gltypes_h_ in our code. Is this necessary? Or would it be sufficient to just protect the typedef's here with the #if !defined(...) line, but not actually #define the #include guards that "belong" to other headers?
Comment 14 Steven Michaud [:smichaud] (Retired) 2011-06-06 15:06:16 PDT
> but it seems wrong for us to be #define'ing __gl_h_ and __gltypes_h_
> in our code. Is this necessary?

I don't know whether or not it's necessary (but note that even
existing code #defines __gl_h_).

> Or would it be sufficient to just protect the typedef's here with
> the #if !defined(...) line, but not actually #define the #include
> guards that "belong" to other headers?

I'd be happy with this, if it worked.  Let me give it a try.

By the way, I'm almost completely unfamiliar with the Mozilla-tree
code that uses OpenGL.  So I'm probably not the best one to judge
whether or not we should #define __gl_h__ and/or __gltypes_h__.
Comment 15 Steven Michaud [:smichaud] (Retired) 2011-06-06 15:17:52 PDT
Bas and Vlad, can either of you explain why the original GLDefs.h code
#defines __gl_h_, in addition to protecting GLDefs.h's definitions of
GLintptr and so forth?
Comment 16 Steven Michaud [:smichaud] (Retired) 2011-06-06 15:49:10 PDT
(Following up comment #14)

>> Or would it be sufficient to just protect the typedef's here with
>> the #if !defined(...) line, but not actually #define the #include >
>> guards that "belong" to other headers?
>
> I'd be happy with this, if it worked.  Let me give it a try.

It doesn't work.  Turns out both __gl_h_ and __gl_types_h_ need to be
#defined.

(Following up comment #15)

> Bas and Vlad, can either of you explain why the original GLDefs.h
> code #defines __gl_h_, in addition to protecting GLDefs.h's
> definitions of GLintptr and so forth?

Never mind -- I can now see why you #defined __gl_h_.

But what do you think of my patch?  Can you think of a better way of
accomplishing its purpose?
Comment 17 Steven Michaud [:smichaud] (Retired) 2011-06-09 14:57:33 PDT
(In further reply to comment #11)

Something like this patch might help build on Meego 1.2, but I think
we should treat that as a separate issue.

In other words, I'd prefer that we keep this bug restricted to
problems encountered building the tree on OS X 10.7 using the 10.7
SDK.
Comment 18 Bas Schouten (:bas.schouten) 2011-06-21 17:30:32 PDT
Comment on attachment 536970 [details] [diff] [review]
Possible fix

Bjacob should probably review this to be sure :)
Comment 19 Benoit Jacob [:bjacob] (mostly away) 2011-06-22 06:01:28 PDT
Comment on attachment 536970 [details] [diff] [review]
Possible fix

The patch is innocuous _if_ it doesn't break the build (needs tryserver).
Comment 20 Steven Michaud [:smichaud] (Retired) 2011-06-22 08:42:18 PDT
Thanks for the review!

> The patch is innocuous _if_ it doesn't break the build (needs
> tryserver).

I ran it through the tryservers (all platforms) on 6-2, and there were
no problems (see comment #10).

Jonathan, do you still have any objections?
Comment 21 Steven Michaud [:smichaud] (Retired) 2011-06-23 07:22:34 PDT
Created attachment 541372 [details] [diff] [review]
Copy of patch for someone else to land
Comment 22 Steven Michaud [:smichaud] (Retired) 2011-06-23 07:24:27 PDT
> Jonathan, do you still have any objections?

I take it that no response means no objections :-)
Comment 23 Jonathan Kew (:jfkthame) 2011-06-23 08:06:10 PDT
(In reply to comment #22)
> > Jonathan, do you still have any objections?
> 
> I take it that no response means no objections :-)

Fair enough! I can't say I _like_ this patch, but I don't have anything better to offer, or any spare cycles to investigate it further, so let's go with whatever works....
Comment 24 Jonathan Kew (:jfkthame) 2011-06-23 08:08:00 PDT
Comment on attachment 536970 [details] [diff] [review]
Possible fix

(Clearing spare r?, as bjacob is much more qualified to review this than I am.)
Comment 25 Steven Michaud [:smichaud] (Retired) 2011-06-30 12:08:35 PDT
Comment on attachment 536970 [details] [diff] [review]
Possible fix

Landed on mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d31497763fdd
Comment 26 Marco Bonardo [::mak] 2011-07-02 01:52:47 PDT
Please don't mark bugs as fixed till they are merged to central.
http://hg.mozilla.org/mozilla-central/rev/d31497763fdd

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