Re: [PATCH] git-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED

From: Kyle J. McKay <mackyle@gmail.com>
Date: Feb 6, 2015, 19:47 UTC
Message-ID: <28F603A7-610B-4711-9D2F-CD9866C96A74@gmail.com>
In-Reply-To: <CAPig+cT0-ftZZyRORx-W2_Nit6XdgrpiyGS=pRjGtHoz1jW+Kg@mail.gmail.com>

On Feb 6, 2015, at 02:00, Eric Sunshine wrote:

Show 19 quoted lines

> On Fri, Feb 6, 2015 at 4:35 AM, Kyle J. McKay <mackyle@gmail.com>  
> wrote:
>> #ifndef NO_OPENSSL
>> +#ifdef __APPLE__
>> #define __AVAILABILITY_MACROS_USES_AVAILABILITY 0
>> -#define MAC_OS_X_VERSION_MIN_REQUIRED MAC_OS_X_VERSION_10_6
>> +#include <AvailabilityMacros.h>
>> +#undef DEPRECATED_ATTRIBUTE
>> +#define DEPRECATED_ATTRIBUTE
>> +#undef __AVAILABILITY_MACROS_USES_AVAILABILITY
>> +#endif
>> #include <openssl/ssl.h>
>> #include <openssl/err.h>
>> -#undef MAC_OS_X_VERSION_MIN_REQUIRED
>> -#undef __AVAILABILITY_MACROS_USES_AVAILABILITY
>
> DEPRECATED_ATTRIBUTE is a fairly generic name. Do we want to be extra
> careful and #undef it here to avoid potential unintended interactions
> with other #includes (Apple or not)?

The new patch only mucks with it when we have #ifdef __APPLE__ and pretty much any apple code is going to end up including the availability stuff sooner or later which manipulates DEPRECATED_ATTRIBUTE. Essentially that makes "DEPRECATED_ATTRIBUTE" a reserved macro name on __APPLE__.

>
>    #ifdef __APPLE__
>    #undef DEPRECATED_ATTRIBUTE
>    #endif

If we do that, won't the compiler then helpfully supply a "0" when the macro is used? Or is that only when an undefined macro is used inside an #if or #elif ?

That would break all later declarations that use it.

> On the other hand, we've already mucked with it, so #undef may be  
> superfluous.

Actually I just tested it. If we #undef it we could end up producing these:

   error: syntax error before ‘DEPRECATED_ATTRIBUTE’

So I think it needs to stay #define'd to nothing to be safe in case anything later on ends up including stuff that uses it. The worst that happens is you don't see some deprecation warnings that you otherwise would. It won't make any functions available that shouldn't be or make unavailable functions that should be.

-Kyle