Summary: | Supporting multiple OS X versions in one binary | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kevin Ollivier <kevino> | ||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED WONTFIX | ||||||||
Severity: | Normal | CC: | ahmad.saleem792, ap, bfulgham, mitz, mrowe | ||||||
Priority: | P2 | Keywords: | Wx | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Kevin Ollivier
2009-10-19 12:27:16 PDT
Created attachment 41439 [details]
Patch to allow compilation using 10.5 SDK on SnowLeopard
As I said, originally I was just trying to compile by using MACOSX_DEPLOYMENT_TARGET=10.5 and specifying the 10.5 SDK but as far as I can tell, this patch creates a binary that works fine on 10.6 as well.
Comment on attachment 41439 [details]
Patch to allow compilation using 10.5 SDK on SnowLeopard
It may appear to work but it will have subtle bugs as *all* of the code wrapped with these macros was written with the assumption that the code within these blocks was targeted for the specific OS version. When building in the manner you mention it will be running on a different version of the OS and may not behave as expected. A number of aspects of the code would likely need to be updated to determine the underlying OS version at runtime and to react accordingly.
It also doesn’t make sense to switch a single use of BUILDING_ON_TIGER and BUILDING_ON_LEOPARD to using availability macros. If we’re going to make this change it needs to be done in a more consistent manner.
(In reply to comment #2) > (From update of attachment 41439 [details]) > It may appear to work but it will have subtle bugs as *all* of the code wrapped > with these macros was written with the assumption that the code within these > blocks was targeted for the specific OS version. When building in the manner > you mention it will be running on a different version of the OS and may not > behave as expected. A number of aspects of the code would likely need to be > updated to determine the underlying OS version at runtime and to react > accordingly. > > It also doesn’t make sense to switch a single use of BUILDING_ON_TIGER and > BUILDING_ON_LEOPARD to using availability macros. If we’re going to make this > change it needs to be done in a more consistent manner. Okay, after doing a grep what I'm seeing is that a vast majority of the use of these macros are in Mac port sources or code blocks, which won't use this support anyway, at least not in the short term. Perhaps that somewhat explains why the change didn't seem so problematic for the wx port. So, how about this? We create a SUPPORTS_<CAT> or <CAT>_COMPATIBLE (where CAT is release name) type of define that corresponds to MIN_REQUIRED for non-Mac ports but for Mac (or any port that wants to keep the existing behavior) will simply be defined to BUILDING_ON_<CAT> for compatibility. The only other thing this would probably require is to add weakly defined symbol checks to allow to use the newer API when running on a newer release, but since wx doesn't use much of the Mac port's Cocoa, CF or CG code, nor presumably would Qt or GTK, I suspect this would actually only need to be done in a few spots. So, does this sound reasonable? If not, any ideas on how else we could approach this issue? Thanks, Kevin (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 41439 [details] [details]) > > It may appear to work but it will have subtle bugs as *all* of the code wrapped > > with these macros was written with the assumption that the code within these > > blocks was targeted for the specific OS version. When building in the manner > > you mention it will be running on a different version of the OS and may not > > behave as expected. A number of aspects of the code would likely need to be > > updated to determine the underlying OS version at runtime and to react > > accordingly. > > > > It also doesn’t make sense to switch a single use of BUILDING_ON_TIGER and > > BUILDING_ON_LEOPARD to using availability macros. If we’re going to make this > > change it needs to be done in a more consistent manner. > > Okay, after doing a grep what I'm seeing is that a vast majority of the use of > these macros are in Mac port sources or code blocks, which won't use this > support anyway, at least not in the short term. Perhaps that somewhat explains > why the change didn't seem so problematic for the wx port. Many of them are Mac-only, yes. ICU is a significant case that is not specific to the Mac version of WebKit, but where the behavior can differ significantly from Tiger’s version to SnowLeopard. We handle this with compile-time checks. > So, how about this? We create a SUPPORTS_<CAT> or <CAT>_COMPATIBLE (where CAT > is release name) type of define that corresponds to MIN_REQUIRED for non-Mac > ports but for Mac (or any port that wants to keep the existing behavior) will > simply be defined to BUILDING_ON_<CAT> for compatibility. The only other thing > this would probably require is to add weakly defined symbol checks to allow to > use the newer API when running on a newer release, but since wx doesn't use > much of the Mac port's Cocoa, CF or CG code, nor presumably would Qt or GTK, I > suspect this would actually only need to be done in a few spots. I can’t easily follow what you’re suggesting here. (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 41439 [details] [details] [details]) > > > It may appear to work but it will have subtle bugs as *all* of the code wrapped > > > with these macros was written with the assumption that the code within these > > > blocks was targeted for the specific OS version. When building in the manner > > > you mention it will be running on a different version of the OS and may not > > > behave as expected. A number of aspects of the code would likely need to be > > > updated to determine the underlying OS version at runtime and to react > > > accordingly. > > > > > > It also doesn’t make sense to switch a single use of BUILDING_ON_TIGER and > > > BUILDING_ON_LEOPARD to using availability macros. If we’re going to make this > > > change it needs to be done in a more consistent manner. > > > > Okay, after doing a grep what I'm seeing is that a vast majority of the use of > > these macros are in Mac port sources or code blocks, which won't use this > > support anyway, at least not in the short term. Perhaps that somewhat explains > > why the change didn't seem so problematic for the wx port. > > Many of them are Mac-only, yes. ICU is a significant case that is not specific > to the Mac version of WebKit, but where the behavior can differ significantly > from Tiger’s version to SnowLeopard. We handle this with compile-time checks. > > > So, how about this? We create a SUPPORTS_<CAT> or <CAT>_COMPATIBLE (where CAT > > is release name) type of define that corresponds to MIN_REQUIRED for non-Mac > > ports but for Mac (or any port that wants to keep the existing behavior) will > > simply be defined to BUILDING_ON_<CAT> for compatibility. The only other thing > > this would probably require is to add weakly defined symbol checks to allow to > > use the newer API when running on a newer release, but since wx doesn't use > > much of the Mac port's Cocoa, CF or CG code, nor presumably would Qt or GTK, I > > suspect this would actually only need to be done in a few spots. > > I can’t easily follow what you’re suggesting here. I'm basically suggesting we add defines such as SUPPORTS_TIGER which would be set as follows #if PLATFORM(MAC) #if BUILDING_ON_TIGER #define SUPPORTS_TIGER // for Apple port, SUPPORTS_TIGER == BUILDING_ON_TIGER #endif #elif PLATFORM(DARWIN) && defined(MAC_OS_X_VERSION_10_4) && MAC_OS_X_MIN_REQUIRED == MAC_OS_X_VERSION_10_4 #define SUPPORTS_TIGER // can be true even under Leopard or Snow Leopard #endif Then say we have the following example code block somewhere in common code: #if BUILDING_ON_TIGER ... 10.4 compatible code ... #elif BUILDING_ON_LEOPARD ... 10.5 compatible code ... #else ... 10.6 compatible code ... #endif It would change to: #if SUPPORTS_TIGER ... 10.4 compatible code ... #elif SUPPORTS_LEOPARD ... 10.5 compatible code ... #else ... 10.6 compatible code ... #endif So the Apple/Mac port would compile this code exactly the same as it does today, because there SUPPORTS_TIGER is only true when BUILDING_ON_TIGER is true, but ports like wx would compile the 10.4 codepath even under 10.5 or 10.6 so long as 10.4 support was set as the min required system. Does this make things clearer? And how does it address the case where you need to do different things based on the OS version? It seems as though you’d need to duplicate code. Created attachment 41603 [details]
Changes to enable a Tiger-SnowLeopard compatible binary for non-Apple ports
I thought it would be easier to produce a patch to make it easier to see the actual code in question, so that we could discuss the cases that need changed in detail. I'm working on setting up a Tiger environment to start testing the binary created by this patch, but I've checked the defines numerous times and to the best of my knowledge this patch does change every define that needs to be changed for a non-Apple port to have multi-OS version compatibility. I didn't mark it for review since it's not polished yet as I may need to make more changes after running some tests on Tiger.
Outside of switching defines, so far I found two places where we would need (or it would be desirable to have) dynamic version checks rather than compile-time checks. One is for a Leopard performance improvement in FrameLoader.cpp, and the other is for the ICU 3.2 code in RenderText.cpp, which I've implemented. The only other cases I could see that might benefit from some sort of dynamic check would be the USE_BACKGROUND_THREAD_TO_SCAVENGE_MEMORY codepath in FastMalloc.cpp (the note suggests we can compile the code on Tiger), and the spelling and grammar changes in Editor.cpp and ContextMenu.cpp, though I wonder if other ports like Qt and GTK would actually using OS X's spelling and grammar support, as with wx we would need to solve the problem for other environments anyway, and if we went that far, we could just use the solution on Mac as well and get Tiger support in the mix.
So, any thoughts/preferences on the above issues? Do you see any particular issues with this patch?
ap@webkit.org - Is this needed anymore, it has Keyword "Wx" but does not seem to be limited to it. Appreciate if you can mark this accordingly. Thanks! We still don't support the use case of embedding one's own copy of Cocoa WebKit in applications, and the wx port is gone. Other ports like Gtk may or may not need this. If they do, tracking with separate bugs would be more appropriate than reviving this wx bug. |