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, 20:43 UTC
Message-ID: <1FC680E6-2258-4DBB-802C-6FB62D29C378@gmail.com>
In-Reply-To: <xmqq7fvuhkl3.fsf@gitster.dls.corp.google.com>

On Feb 6, 2015, at 12:05, Junio C Hamano wrote:

Show 23 quoted lines

> "Kyle J. McKay" <mackyle@gmail.com> writes:
>
>> 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.
>
> Doesn't the fact that your test failed indicates that it is not jsut
> "to be safe in case" but is required for correctness?
>
> The first hit for "MAC_OS_X_VERSION_MIN_REQUIRED" was this:
>
>  https://opensource.apple.com/source/CarbonHeaders/CarbonHeaders-18.1/AvailabilityMacros.h
>
> which marks quite a many macros to that value.  I do not know what
> changes they make to openssl/*.h (which is included just after the
> above header is included, but I would imagine that is where the
> AVAILABLE_MAC_OS_X_VERSION_XXX_AND_LATER_BUT_DEPRECATED_IN_MAC_OS_VERSION_YYY
> macros are checked and annoying warnings that are being squelched by
> the previous change are given?

Yes.

Although Eric didn't specify exactly where when he suggested adding this:

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

>    #ifdef __APPLE__
>    #undef DEPRECATED_ATTRIBUTE
>    #endif

I took the suggestion to be after the openssl/*.h headers are included which would avoid the error of having DEPRECATED_ATTRIBUTE be #undef'd for them. But, even math.h can end up including AvailabilityMacros.h, so I think #undef'ing DEPRECATED_ATTRIBUTE after the openssl/*.h headers are included would be unsafe in general. While we might happen to get away with that today, if say compat/apple-common- crypto.h changes in the future (or for that matter any sequence of includes in other files or any headers in the Apple SDK) we could start seeing the error.

TLDR; yeah, DEPRECATED_ATTRIBUTE needs to remain defined to nothing.

-Kyle