Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X

On Fri, May 17, 2013 at 4:21 AM, David Aguilar [off-list ref] wrote:

On Thu, May 16, 2013 at 11:18 PM, Eric Sunshine [off-list ref] wrote:

quoted

On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen [off-list ref] wrote:

quoted

On 2013-05-15 09.11, David Aguilar wrote:

quoted

+     ifndef NO_APPLE_COMMON_CRYPTO
+             APPLE_COMMON_CRYPTO = YesPlease
+     endif
      NO_REGEX = YesPlease
      PTHREAD_LIBS =
 endif

@@ -1389,10 +1396,16 @@ ifdef PPC_SHA1
      LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
      LIB_H += ppc/sha1.h
 else
+ifdef APPLE_COMMON_CRYPTO
+     BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+     SHA1_HEADER = <CommonCrypto/CommonDigest.h>

Would it make sense to replace APPLE_COMMON_CRYPTO
with COMMON_DIGEST_FOR_OPENSSL ?

In the spirit of other Makefile-defines becoming Compiler defines,
a random picked example:
ifdef NO_STRTOULL
        COMPAT_CFLAGS += -DNO_STRTOULL
endif

Not necessarily. Unlike NO_STRTOULL and cousins,
COMMON_DIGEST_FOR_OPENSSL is not a Git build tweak; it is merely a
(public) implementation detail of the Apple header [1] to magically
associate OpenSSL digest functions with CommonCrypto counterparts.
It's not the only such macro recognized by the Apple headers. For
instance, COMMON_DIGEST_FOR_RFC_1321 magically associates legacy MD5
digest functions with CommonCrypto counterparts.

Further, as Junio noted elsewhere, David is using CommonCrypto for
HMAC replacements, not just for digest replacements, so a Makefile
knob with DIGEST in its name is not really appropriate. More
generally, David would like to find CommonCrypto replacements for all
the OpenSSL functionality, so a Makefile knob named after DIGEST is
too specific.

These considerations motivated the original suggestion for a single
Git Makefile knob to enable/disable, as a unit, all CommonCrypto
replacements. Such a knob would naturally have COMMON_CRYPTO as part
of its name.

This is a nice justification for taking v5 of this series over v6.

You will consider this bike-shedding (I don't), but the above also is
good justification for revising your HMAC patch to _not_ rely on
COMMON_DIGEST_FOR_OPENSSL, which is an implementation detail of your
SHA patch, rather than a proper build knob.

Similar to NO_STRTOULL and cousins, you should have a #define (such as
NO_APPLE_COMMON_CRYPTO or NO_COMMON_CRYPTO) which is consulted by your
HMAC patch and any future patches you submit to map CommonCrypto
counterparts to OpenSSL functions. The fact that you also must #define
COMMON_DIGEST_FOR_OPENSSL for the SHA patch is just an implementation
detail of that one patch; it is not relevant to the other patches.

-- ES