Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Mentovai]

Mark Rowe wrote:

> It'd be complicated to do this more widely since the AvailabilityMacros.h
> version of the macros are defined even when building for iOS. We'd need to
> review all of the uses to ensure that they were handled correctly. Given
> that I think it'd be better to just use this as a localized workaround
> until we drop support for building for Leopard.
>

The one place where we found it to be significant so far is in a
Chromium-specific file that’s not relevant to iOS, and we’re just going to
use MAC_OS_X_VERSION_MAX_ALLOWED in that one instance for the time being.
If it happens anywhere else with this SDK, it’ll likely also be
Chromium-specific, and we’ll just deal with the ugliness of having to use a
different macro for this purpose until we get rid of the 10.5 SDK
altogether. Shouldn’t be long.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Rowe]

On 2012-07-12, at 11:29, Mark Mentovai  wrote:

> We just discovered (via a rollout on bug 91103) that there’s a bug with 
> __MAC_OS_X_VERSION_MAX_ALLOWED in the version of the 10.5 SDK as present in 
> Xcode 3.2.6. This may be the last version of the 10.5 SDK ever released (I 
> haven’t checked all of the early Xcode 4 releases). Chromium uses this SDK 
> for its builds, although will likely switch to the 10.6 SDK in the near 
> future.
> 
> In short, __MAC_OS_X_VERSION_MAX_ALLOWED, which is supposed to track the SDK 
> version, is set to 1060 instead of the expected 1050 in this version of the 
> 10.5 SDK.

This is unfortunate. My understanding is that Chromium will soon be dropping 
Leopard support so this problem should go away in the near future.

> At least for the purposes of testing whether the 10.5 SDK is in use, we’ll 
> need to use ’s MAC_OS_X_VERSION_MAX_ALLOWED instead of 
> ’s __MAC_OS_X_VERSION_MAX_ALLOWED. I guess it’s a question of 
> style whether the rest of WebKit should follow.

It'd be complicated to do this more widely since the AvailabilityMacros.h 
version of the macros are defined even when building for iOS. We'd need to 
review all of the uses to ensure that they were handled correctly. Given that I 
think it'd be better to just use this as a localized workaround until we drop 
support for building for Leopard.

An alternative workaround would be to explicitly define 
__MAC_OS_X_VERSION_MAX_ALLOWED == 1050 in your compiler invocation when 
building against the 10.5 SDK.

- Mark

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Mentovai]

We just discovered (via a rollout on bug 91103) that there’s a bug
with __MAC_OS_X_VERSION_MAX_ALLOWED in the version of the 10.5 SDK as
present in Xcode 3.2.6. This may be the last version of the 10.5 SDK ever
released (I haven’t checked all of the early Xcode 4 releases). Chromium
uses this SDK for its builds, although will likely switch to the 10.6 SDK
in the near future.

In short, __MAC_OS_X_VERSION_MAX_ALLOWED, which is supposed to track the
SDK version, is set to 1060 instead of the expected 1050 in this version of
the 10.5 SDK.

--
// clang t.c -isysroot /Xcode3.2/SDKs/MacOSX10.5.sdk
-mmacosx-version-min=10.4 -o t
#include 
#include 
#include 

int main(int argc, char* argv[]) {
#define PRINT_MACRO(m) printf("%s = %d\n", # m, m)
  PRINT_MACRO(MAC_OS_X_VERSION_MIN_REQUIRED);
  PRINT_MACRO(MAC_OS_X_VERSION_MAX_ALLOWED);
  PRINT_MACRO(__MAC_OS_X_VERSION_MIN_REQUIRED);
  PRINT_MACRO(__MAC_OS_X_VERSION_MAX_ALLOWED);
#undef PRINT_MACRO
  return 0;
}
--

produces

MAC_OS_X_VERSION_MIN_REQUIRED = 1040
MAC_OS_X_VERSION_MAX_ALLOWED = 1050
__MAC_OS_X_VERSION_MIN_REQUIRED = 1040
__MAC_OS_X_VERSION_MAX_ALLOWED = 1060

Oops.

I traced this problem to , which in this SDK, says
(slightly abbreviated):

#elif defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__)
#ifndef __MAC_OS_X_VERSION_MAX_ALLOWED
#define __MAC_OS_X_VERSION_MAX_ALLOWED __MAC_10_6
#endif
#endif

At least for the purposes of testing whether the 10.5 SDK is in use, we’ll
need to use ’s MAC_OS_X_VERSION_MAX_ALLOWED instead
of ’s __MAC_OS_X_VERSION_MAX_ALLOWED. I guess it’s a
question of style whether the rest of WebKit should follow.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Mentovai]

Mark Rowe wrote:

> TN2064 appears to have been last modified in 2003, so it predates the
> existence of Availability.h.
>

Well, that’s about as long as I’ve been writing SDK- and
deployment-target-aware code…

Availability.h was introduced with the iOS SDK in order to support
> availability macros that are compatible with both OS X and iOS, where the
> older macros from AvailabilityMacros.h were specific to OS X.
>

Cool. Other than double-underscore paranoia, there’s no real difference or
complaint from me.

I guess the one drawback shared by ,
, and the outgoing BUILDING_ON_*/TARGETING_* macros
is that if you forget to #include the right header, the preprocessor
silently does something that you’re probably not expecting. Using a
function-like macro like USE(), OS(), and PLATFORM() would help.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Rowe]

On 2012-07-11, at 17:46, Mark Mentovai  wrote:

> Tony brought me in to comment on what impact this might have on the Chromium 
> Mac build. It shouldn’t have any impact. Any use of the compiler-defined 
> macros is fine.

I agree. The only impact I expect this to have is if I've missed hand-fixing 
any cases that were using BUILDING_ON and meant it (vs the vast majority of 
cases where they really wanted TARGETING). There were around half a dozen 
instances of this that I noticed and fixed when building the Mac build against 
an SDK but it's possible some may exist in Chromium-specific code paths. These 
cases should cause build failures and be obvious to address.

> In Chrome code, we usually use MAC_OS_X_VERSION_MAX_ALLOWED and 
> MAC_OS_X_VERSION_MIN_REQUIRED from , along with 
> symbolic macros like MAC_OS_X_VERSION_10_7 instead of 1070. This has an 
> annoying disadvantage that older SDKs (like the 10.6 SDK) don’t define 
> MAC_OS_X_VERSION_10_7, so we’re stuck testing things like 
> defined(MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 
> MAC_OS_X_VERSION_10_7. The same applies when using , where 
> the symbolic names are of the form __MAC_10_7, but you seem to have chosen 
> not to use those, thus condensing the macro logic.
> 
> We used ’s MAC_OS_X_VERSION_MAX_ALLOWED and 
> MAC_OS_X_VERSION_MIN_REQUIRED instead of ’s 
> __MAC_OS_X_VERSION_MAX_ALLOWED and __MAC_OS_X_VERSION_MIN_REQUIRED because 
> the latter header’s comments discuss how it’s appropriate for OS headers, 
> while the former documents its use for user (non-OS) code. TN2064 also 
> recommends .

Availability.h has the following to say which addresses both of these points:

It is also possible to use the *_VERSION_MIN_REQUIRED in source code to 
make one
source base that can be compiled to target a range of OS versions.  It is 
best
to not use the _MAC_* and __IPHONE_* macros for comparisons, but rather 
their values.
That is because you might get compiled on an old OS that does not define a 
later
OS version macro, and in the C preprocessor undefined values evaluate to 
zero
in expresssions, which could cause the #if expression to evaluate in an 
unexpected
way.

TN2064 appears to have been last modified in 2003, so it predates the existence 
of Availability.h. Availability.h was introduced with the iOS SDK in order to 
support availability macros that are compatible with both OS X and iOS, where 
the older macros from AvailabilityMacros.h were specific to OS X.

> I never liked the WebKit-specific BUILDING_ON_*/TARGETING_* macros and am 
> happy to see them go.
> 
> My one real gripe with the macros from both  and 
>  is that it’s not terribly obvious which one refers to 
> the SDK (it’s MAX_ALLOWED) and which refers to the deployment target (it’s 
> MIN_REQUIRED). Until you’re familiar with the macros, I think it’s unclear 
> that “allowed” refers to APIs allowed to be called because they’re present in 
> the SDK, and unclear that “required” refers to the runtime requirement. This 
> probably leads to more misuse and abuse than if they had SDK and DT in their 
> names. I guess BUILDING_ON_* and TARGETING_* were a little better in that 
> regard, but they were misused and abused too. SDKs and DTs are probably just 
> too confusing to begin with.

I agree that the terminology used is somewhat confusing. I don't think it's a 
huge concern though since the minimum required OS version is what the vast 
majority of the checks within WebKit care about, and that concept is quite easy 
to understand.

Thanks for taking the time to look this over!

- Mark

> 
> 
> On Tue, Jul 10, 2012 at 7:24 PM, Mark Rowe  wrote:
> I would like to propose removing the BUILDING_ON and TARGETING family of 
> macros that are used to build code conditionally for different versions of OS 
> X. I propose this in order to address several problems:
> 
> The checks are verbose, and getting worse.
> 
> For instance, in order to write code targeting OS X 10.8 and newer I have to 
> enumerate all other supported OS versions:
> 
> #if !defined(TARGETING_LEOPARD) && !defined(TARGETING_SNOW_LEOPARD) && 
> !defined(TARGETING_LION)
> …
> #endif
> 
> This problem has become worse over time as the number of supported OS X 
> versions in the WebKit code base has increased.
> 
> The nature of the version checks are often not obvious at first glance.
> 
> In order to understand the checks you have to first remember the marketing 
> names of the various OS X releases. You must then reason about the 
> conditional logic of the check itself, which will often contains multiple 
> negated clauses.
> 
> Almost all current uses are incorrect in the context of SDKs.
> 
> The vast majority of the checks in WebKit use the BUILDING_ON macros where 
> the TARGETING macros would be more appropriate. This hasn't cause many 
> problems to date since builds of WebKit on OS X for the most part do not use 
> SDKs. This means that they build with __MAC_OS_X

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Mentovai]

Tony brought me in to comment on what impact this might have on the
Chromium Mac build. It shouldn’t have any impact. Any use of the
compiler-defined macros is fine.

In Chrome code, we usually use MAC_OS_X_VERSION_MAX_ALLOWED and
MAC_OS_X_VERSION_MIN_REQUIRED from , along with
symbolic macros like MAC_OS_X_VERSION_10_7 instead of 1070. This has an
annoying disadvantage that older SDKs (like the 10.6 SDK) don’t define
MAC_OS_X_VERSION_10_7, so we’re stuck testing things like
defined(MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >=
MAC_OS_X_VERSION_10_7. The same applies when using , where
the symbolic names are of the form __MAC_10_7, but you seem to have chosen
not to use those, thus condensing the macro logic.

We used ’s MAC_OS_X_VERSION_MAX_ALLOWED and
MAC_OS_X_VERSION_MIN_REQUIRED instead of ’s
__MAC_OS_X_VERSION_MAX_ALLOWED and __MAC_OS_X_VERSION_MIN_REQUIRED because
the latter header’s comments discuss how it’s appropriate for OS headers,
while the former documents its use for user (non-OS) code. TN2064 also
recommends .

I never liked the WebKit-specific BUILDING_ON_*/TARGETING_* macros and am
happy to see them go.

My one real gripe with the macros from both  and
 is that it’s not terribly obvious which one refers
to the SDK (it’s MAX_ALLOWED) and which refers to the deployment target
(it’s MIN_REQUIRED). Until you’re familiar with the macros, I think it’s
unclear that “allowed” refers to APIs allowed to be called because they’re
present in the SDK, and unclear that “required” refers to the runtime
requirement. This probably leads to more misuse and abuse than if they had
SDK and DT in their names. I guess BUILDING_ON_* and TARGETING_* were a
little better in that regard, but they were misused and abused too. SDKs
and DTs are probably just too confusing to begin with.


On Tue, Jul 10, 2012 at 7:24 PM, Mark Rowe  wrote:

> I would like to propose removing the BUILDING_ON and TARGETING family of
> macros that are used to build code conditionally for different versions of
> OS X. I propose this in order to address several problems:
>
>
>- The checks are verbose, and getting worse.
>
>
> For instance, in order to write code targeting OS X 10.8 and newer I have
> to enumerate all other supported OS versions:
>
> #if !defined(TARGETING_LEOPARD) && !defined(TARGETING_SNOW_LEOPARD) &&
> !defined(TARGETING_LION)
> …
> #endif
>
>
> This problem has become worse over time as the number of supported OS X
> versions in the WebKit code base has increased.
>
>
>
>- The nature of the version checks are often not obvious at first
>glance.
>
>
> In order to understand the checks you have to first remember the marketing
> names of the various OS X releases. You must then reason about the
> conditional logic of the check itself, which will often contains multiple
> negated clauses.
>
>
>
>- Almost all current uses are incorrect in the context of SDKs.
>
>
> The vast majority of the checks in WebKit use the BUILDING_ON macros where
> the TARGETING macros would be more appropriate. This hasn't cause many
> problems to date since builds of WebKit on OS X for the most part do not
> use SDKs. This means that they build with __MAC_OS_X_VERSION_MIN_REQUIRED
> == __MAC_OS_X_VERSION_MAX_ALLOWED, resulting in the BUILDING_ON and
> TARGETING macros being equivalent. However, when building with a deployment
> target of an older OS release than the SDK you're building against, the
> BUILDING_ON and TARGETING macros will have different behavior. The result
> is that WebKit fails to build against an SDK when targeting an older OS
> release.
>
>
> My proposed solution to these problems is to remove the BUILDING_ON and
> TARGETING macros. The vast majority of the BUILDING_ON uses and all of the
> TARGETING uses would be replaced with tests against
> __MAC_OS_X_VERSION_MIN_REQUIRED. The small number of uses of BUILDING_ON
> that are being used correctly would be replaced with tests against
> __MAC_OS_X_VERSION_MAX_ALLOWED.
>
> The example above of code targeting OS X 10.8 and newer would become:
>
> #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080
> …
> #endif
>
> Code that wishes to target only OS X 10.6 and older would become:
>
> #if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1060
> …
> #endif
>
> This is much more concise and understandable than the current approach.
>
>
> I'm open to feedback on this proposal, but I'd like to move forward with
> this change in the next day or two if no one objects.
>
> - Mark
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Rowe]

On 2012-07-10, at 16:24, Mark Rowe  wrote:

> I'm open to feedback on this proposal, but I'd like to move forward with this 
> change in the next day or two if no one objects.

Given the lack of outcry I've posted patches on 
 that make this change.

- Mark


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Adam Barth]

Tony and Mark (Mentovai) would know more, but I believe the Chromium port
uses these macros in order to build against a specific SDK.  For example,
the Chromium port currently builds against the 10.5 SDK.  (This is slated
to change shortly as the Chromium project has dropped support for 10.5.)

Adam


On Tue, Jul 10, 2012 at 4:24 PM, Mark Rowe  wrote:

> I would like to propose removing the BUILDING_ON and TARGETING family of
> macros that are used to build code conditionally for different versions of
> OS X. I propose this in order to address several problems:
>
>
>- The checks are verbose, and getting worse.
>
>
> For instance, in order to write code targeting OS X 10.8 and newer I have
> to enumerate all other supported OS versions:
>
> #if !defined(TARGETING_LEOPARD) && !defined(TARGETING_SNOW_LEOPARD) &&
> !defined(TARGETING_LION)
> …
> #endif
>
>
> This problem has become worse over time as the number of supported OS X
> versions in the WebKit code base has increased.
>
>
>
>- The nature of the version checks are often not obvious at first
>glance.
>
>
> In order to understand the checks you have to first remember the marketing
> names of the various OS X releases. You must then reason about the
> conditional logic of the check itself, which will often contains multiple
> negated clauses.
>
>
>
>- Almost all current uses are incorrect in the context of SDKs.
>
>
> The vast majority of the checks in WebKit use the BUILDING_ON macros where
> the TARGETING macros would be more appropriate. This hasn't cause many
> problems to date since builds of WebKit on OS X for the most part do not
> use SDKs. This means that they build with __MAC_OS_X_VERSION_MIN_REQUIRED
> == __MAC_OS_X_VERSION_MAX_ALLOWED, resulting in the BUILDING_ON and
> TARGETING macros being equivalent. However, when building with a deployment
> target of an older OS release than the SDK you're building against, the
> BUILDING_ON and TARGETING macros will have different behavior. The result
> is that WebKit fails to build against an SDK when targeting an older OS
> release.
>
>
> My proposed solution to these problems is to remove the BUILDING_ON and
> TARGETING macros. The vast majority of the BUILDING_ON uses and all of the
> TARGETING uses would be replaced with tests against
> __MAC_OS_X_VERSION_MIN_REQUIRED. The small number of uses of BUILDING_ON
> that are being used correctly would be replaced with tests against
> __MAC_OS_X_VERSION_MAX_ALLOWED.
>
> The example above of code targeting OS X 10.8 and newer would become:
>
> #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080
> …
> #endif
>
> Code that wishes to target only OS X 10.6 and older would become:
>
> #if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1060
> …
> #endif
>
> This is much more concise and understandable than the current approach.
>
>
> I'm open to feedback on this proposal, but I'd like to move forward with
> this change in the next day or two if no one objects.
>
> - Mark
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Rowe]

I would like to propose removing the BUILDING_ON and TARGETING family of macros 
that are used to build code conditionally for different versions of OS X. I 
propose this in order to address several problems:

The checks are verbose, and getting worse.

For instance, in order to write code targeting OS X 10.8 and newer I have to 
enumerate all other supported OS versions:

#if !defined(TARGETING_LEOPARD) && !defined(TARGETING_SNOW_LEOPARD) && 
!defined(TARGETING_LION)
…
#endif

This problem has become worse over time as the number of supported OS X 
versions in the WebKit code base has increased.

The nature of the version checks are often not obvious at first glance.

In order to understand the checks you have to first remember the marketing 
names of the various OS X releases. You must then reason about the conditional 
logic of the check itself, which will often contains multiple negated clauses.

Almost all current uses are incorrect in the context of SDKs.

The vast majority of the checks in WebKit use the BUILDING_ON macros where the 
TARGETING macros would be more appropriate. This hasn't cause many problems to 
date since builds of WebKit on OS X for the most part do not use SDKs. This 
means that they build with __MAC_OS_X_VERSION_MIN_REQUIRED == 
__MAC_OS_X_VERSION_MAX_ALLOWED, resulting in the BUILDING_ON and TARGETING 
macros being equivalent. However, when building with a deployment target of an 
older OS release than the SDK you're building against, the BUILDING_ON and 
TARGETING macros will have different behavior. The result is that WebKit fails 
to build against an SDK when targeting an older OS release.

My proposed solution to these problems is to remove the BUILDING_ON and 
TARGETING macros. The vast majority of the BUILDING_ON uses and all of the 
TARGETING uses would be replaced with tests against 
__MAC_OS_X_VERSION_MIN_REQUIRED. The small number of uses of BUILDING_ON that 
are being used correctly would be replaced with tests against 
__MAC_OS_X_VERSION_MAX_ALLOWED.

The example above of code targeting OS X 10.8 and newer would become:

#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080
…
#endif

Code that wishes to target only OS X 10.6 and older would become:

#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1060
…
#endif

This is much more concise and understandable than the current approach.


I'm open to feedback on this proposal, but I'd like to move forward with this 
change in the next day or two if no one objects.

- Mark

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev