* [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X @ 2013-05-15 7:11 David Aguilar 2013-05-15 7:11 ` [PATCH v5 2/2] imap-send: eliminate HMAC " David Aguilar 2013-05-15 17:56 ` [PATCH v5 1/2] cache.h: eliminate SHA-1 " Torsten Bögershausen 0 siblings, 2 replies; 13+ messages in thread From: David Aguilar @ 2013-05-15 7:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jonathan Nieder, Eric Sunshine Mac OS X 10.8 Mountain Lion prints warnings when building git: warning: 'SHA1_Init' is deprecated (declared at /usr/include/openssl/sha.h:121) Silence the warnings by using the CommonCrytpo SHA-1 functions for SHA1_Init(), SHA1_Update(), and SHA1_Final(). COMMON_DIGEST_FOR_OPENSSL is defined to enable the OpenSSL compatibility macros in CommonDigest.h. Add a NO_APPLE_COMMON_CRYPTO option to the Makefile to allow users to opt out of using this library. When defined, Git will use OpenSSL instead. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: David Aguilar <davvid@gmail.com> --- Both of these are replacement patches "pu". Changes from last time: It now uses a single APPLE_COMMON_CRYPTO definition. Users can now opt-out by setting NO_APPLE_COMMON_CRYPTO. Makefile | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Makefile b/Makefile index f698c1a..8309c41 100644 --- a/Makefile +++ b/Makefile @@ -137,6 +137,10 @@ all:: # specify your own (or DarwinPort's) include directories and # library directories by defining CFLAGS and LDFLAGS appropriately. # +# Define NO_APPLE_COMMON_CRYPTO if you are building on Darwin/Mac OS X +# and do not want to use Apple's CommonCrypto library. This allows you +# to provide your own OpenSSL library, for example from MacPorts. +# # Define BLK_SHA1 environment variable to make use of the bundled # optimized C SHA1 routine. # @@ -1054,6 +1058,9 @@ ifeq ($(uname_S),Darwin) BASIC_LDFLAGS += -L/opt/local/lib endif endif + 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> +else SHA1_HEADER = <openssl/sha.h> EXTLIBS += $(LIB_4_CRYPTO) endif endif +endif + ifdef NO_PERL_MAKEMAKER export NO_PERL_MAKEMAKER endif -- 1.8.3.rc2.2.gbc955d1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 2/2] imap-send: eliminate HMAC deprecation warnings on Mac OS X 2013-05-15 7:11 [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X David Aguilar @ 2013-05-15 7:11 ` David Aguilar 2013-05-15 17:56 ` [PATCH v5 1/2] cache.h: eliminate SHA-1 " Torsten Bögershausen 1 sibling, 0 replies; 13+ messages in thread From: David Aguilar @ 2013-05-15 7:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jonathan Nieder, Eric Sunshine Mac OS X 10.8 Mountain Lion warns that HMAC_Init() and friends are deprecated. Detect the COMMON_CRYPTO_FOR_OPENSSL definition and use CommonCrypto's HMAC functions to eliminate the warnings. Signed-off-by: David Aguilar <davvid@gmail.com> --- Changes since last time: This version re-uses the existing COMMON_CRYPTO_FOR_OPENSSL define instead of tweaking the Makefile to add a new one, so it's simpler. My previous patch had Jonathan's reviewed-by tag, but he hasn't reviewed this exact patch, so I removed it. The C macros are unchanged. imap-send.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/imap-send.c b/imap-send.c index d9bcfb4..96012b1 100644 --- a/imap-send.c +++ b/imap-send.c @@ -29,8 +29,18 @@ #ifdef NO_OPENSSL typedef void *SSL; #else +#ifdef COMMON_DIGEST_FOR_OPENSSL +#include <CommonCrypto/CommonHMAC.h> +#define HMAC_CTX CCHmacContext +#define HMAC_Init(hmac, key, len, algo) CCHmacInit(hmac, algo, key, len) +#define HMAC_Update CCHmacUpdate +#define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash) +#define HMAC_CTX_cleanup +#define EVP_md5() kCCHmacAlgMD5 +#else #include <openssl/evp.h> #include <openssl/hmac.h> +#endif #include <openssl/x509v3.h> #endif -- 1.8.3.rc2.2.gbc955d1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-15 7:11 [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X David Aguilar 2013-05-15 7:11 ` [PATCH v5 2/2] imap-send: eliminate HMAC " David Aguilar @ 2013-05-15 17:56 ` Torsten Bögershausen 2013-05-17 6:18 ` Eric Sunshine 1 sibling, 1 reply; 13+ messages in thread From: Torsten Bögershausen @ 2013-05-15 17:56 UTC (permalink / raw) To: David Aguilar; +Cc: Junio C Hamano, git, Jonathan Nieder, Eric Sunshine On 2013-05-15 09.11, David Aguilar wrote: > Mac OS X 10.8 Mountain Lion prints warnings when building git: > > warning: 'SHA1_Init' is deprecated > (declared at /usr/include/openssl/sha.h:121) > > Silence the warnings by using the CommonCrytpo SHA-1 > functions for SHA1_Init(), SHA1_Update(), and SHA1_Final(). > > COMMON_DIGEST_FOR_OPENSSL is defined to enable the OpenSSL > compatibility macros in CommonDigest.h. > > Add a NO_APPLE_COMMON_CRYPTO option to the Makefile to allow > users to opt out of using this library. When defined, Git will > use OpenSSL instead. > > Helped-by: Eric Sunshine <sunshine@sunshineco.com> > Signed-off-by: David Aguilar <davvid@gmail.com> > --- > Both of these are replacement patches "pu". > > Changes from last time: > > It now uses a single APPLE_COMMON_CRYPTO definition. > Users can now opt-out by setting NO_APPLE_COMMON_CRYPTO. > > Makefile | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/Makefile b/Makefile > index f698c1a..8309c41 100644 > --- a/Makefile > +++ b/Makefile > @@ -137,6 +137,10 @@ all:: > # specify your own (or DarwinPort's) include directories and > # library directories by defining CFLAGS and LDFLAGS appropriately. > # > +# Define NO_APPLE_COMMON_CRYPTO if you are building on Darwin/Mac OS X > +# and do not want to use Apple's CommonCrypto library. This allows you > +# to provide your own OpenSSL library, for example from MacPorts. > +# > # Define BLK_SHA1 environment variable to make use of the bundled > # optimized C SHA1 routine. > # > @@ -1054,6 +1058,9 @@ ifeq ($(uname_S),Darwin) > BASIC_LDFLAGS += -L/opt/local/lib > endif > endif > + 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 /Torsten ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-15 17:56 ` [PATCH v5 1/2] cache.h: eliminate SHA-1 " Torsten Bögershausen @ 2013-05-17 6:18 ` Eric Sunshine 2013-05-17 8:21 ` David Aguilar 0 siblings, 1 reply; 13+ messages in thread From: Eric Sunshine @ 2013-05-17 6:18 UTC (permalink / raw) To: Torsten Bögershausen Cc: David Aguilar, Junio C Hamano, Git List, Jonathan Nieder On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen <tboegi@web.de> wrote: > On 2013-05-15 09.11, David Aguilar wrote: >> + 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. [1]: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/usr/include/CommonCrypto/CommonDigest.h -- ES ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-17 6:18 ` Eric Sunshine @ 2013-05-17 8:21 ` David Aguilar 2013-05-17 16:53 ` Junio C Hamano 2013-05-17 17:40 ` Eric Sunshine 0 siblings, 2 replies; 13+ messages in thread From: David Aguilar @ 2013-05-17 8:21 UTC (permalink / raw) To: Eric Sunshine Cc: Torsten Bögershausen, Junio C Hamano, Git List, Jonathan Nieder On Thu, May 16, 2013 at 11:18 PM, Eric Sunshine <sunshine@sunshineco.com> wrote: > On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen <tboegi@web.de> wrote: >> On 2013-05-15 09.11, David Aguilar wrote: >>> + 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. > > [1]: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/usr/include/CommonCrypto/CommonDigest.h This is a nice justification for taking v5 of this series over v6. Sorry for all the churn in this series, Junio. I wrote v5 so I certainly felt it was a good idea at the time, and I feel bad for not having waited longer before sending out v6 (which is what was eventually queued in "pu"). Do you have advice on how we should proceed? :sigh: sorry for wasting so much maintainer time on this series already. If you need any resends or anything please let me know. This time I'll wait for a strong opinion before firing off patches. My opinion: yeah, v5's ([NO_]APPLE_COMMON_CRYPTO) was probably where we should have stopped painting. Hindsight is 20/20. Luckily it never left "pu". -- David ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-17 8:21 ` David Aguilar @ 2013-05-17 16:53 ` Junio C Hamano 2013-05-17 17:20 ` David Aguilar 2013-05-17 17:29 ` Eric Sunshine 2013-05-17 17:40 ` Eric Sunshine 1 sibling, 2 replies; 13+ messages in thread From: Junio C Hamano @ 2013-05-17 16:53 UTC (permalink / raw) To: David Aguilar Cc: Eric Sunshine, Torsten Bögershausen, Git List, Jonathan Nieder David Aguilar <davvid@gmail.com> writes: > Do you have advice on how we should proceed? :sigh: sorry for wasting > so much maintainer time on this series already. If you need any > resends or anything please let me know. This time I'll wait for a > strong opinion before firing off patches. > > My opinion: yeah, v5's ([NO_]APPLE_COMMON_CRYPTO) was probably where > we should have stopped painting. Hindsight is 20/20. Luckily it > never left "pu". I could do this easily: $ git checkout da/darwin ;# b72ac20a6f73b $ git format-patch --stdout -2 | sed -e 's|COMMON_DIGEST_FOR_OPENSSL|APPLE_COMMON_CRYPTO|g' >P.mbox $ git checkout HEAD^^ ;# 29de20504e $ git am P.mbox $ git diff da/darwin HEAD ;# sanity check $ git log da/darwin.. ;# sanity check $ git branch -f da/darwin if you nicely ask ;-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-17 16:53 ` Junio C Hamano @ 2013-05-17 17:20 ` David Aguilar 2013-05-17 17:29 ` Eric Sunshine 1 sibling, 0 replies; 13+ messages in thread From: David Aguilar @ 2013-05-17 17:20 UTC (permalink / raw) To: Junio C Hamano Cc: Eric Sunshine, Torsten Bögershausen, Git List, Jonathan Nieder On Fri, May 17, 2013 at 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote: > David Aguilar <davvid@gmail.com> writes: > >> Do you have advice on how we should proceed? :sigh: sorry for wasting >> so much maintainer time on this series already. If you need any >> resends or anything please let me know. This time I'll wait for a >> strong opinion before firing off patches. >> >> My opinion: yeah, v5's ([NO_]APPLE_COMMON_CRYPTO) was probably where >> we should have stopped painting. Hindsight is 20/20. Luckily it >> never left "pu". > > I could do this easily: > > $ git checkout da/darwin ;# b72ac20a6f73b > $ git format-patch --stdout -2 | > sed -e 's|COMMON_DIGEST_FOR_OPENSSL|APPLE_COMMON_CRYPTO|g' >P.mbox > $ git checkout HEAD^^ ;# 29de20504e > $ git am P.mbox > $ git diff da/darwin HEAD ;# sanity check > $ git log da/darwin.. ;# sanity check > $ git branch -f da/darwin > > if you nicely ask ;-) gitster please ;-) -- David ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-17 16:53 ` Junio C Hamano 2013-05-17 17:20 ` David Aguilar @ 2013-05-17 17:29 ` Eric Sunshine 2013-05-17 17:57 ` Junio C Hamano 1 sibling, 1 reply; 13+ messages in thread From: Eric Sunshine @ 2013-05-17 17:29 UTC (permalink / raw) To: Junio C Hamano Cc: David Aguilar, Torsten Bögershausen, Git List, Jonathan Nieder On Fri, May 17, 2013 at 12:53 PM, Junio C Hamano <gitster@pobox.com> wrote: > David Aguilar <davvid@gmail.com> writes: > >> Do you have advice on how we should proceed? :sigh: sorry for wasting >> so much maintainer time on this series already. If you need any >> resends or anything please let me know. This time I'll wait for a >> strong opinion before firing off patches. >> >> My opinion: yeah, v5's ([NO_]APPLE_COMMON_CRYPTO) was probably where >> we should have stopped painting. Hindsight is 20/20. Luckily it >> never left "pu". > > I could do this easily: > > $ git checkout da/darwin ;# b72ac20a6f73b > $ git format-patch --stdout -2 | > sed -e 's|COMMON_DIGEST_FOR_OPENSSL|APPLE_COMMON_CRYPTO|g' >P.mbox > $ git checkout HEAD^^ ;# 29de20504e > $ git am P.mbox > $ git diff da/darwin HEAD ;# sanity check > $ git log da/darwin.. ;# sanity check > $ git branch -f da/darwin That would lose the one legitimate COMMON_DIGEST_FOR_OPENSSL which needs to be defined before <CommonCrypto/CommonDigest.h> is included. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-17 17:29 ` Eric Sunshine @ 2013-05-17 17:57 ` Junio C Hamano 2013-05-18 0:38 ` David Aguilar 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2013-05-17 17:57 UTC (permalink / raw) To: Eric Sunshine Cc: David Aguilar, Torsten Bögershausen, Git List, Jonathan Nieder Eric Sunshine <sunshine@sunshineco.com> writes: > On Fri, May 17, 2013 at 12:53 PM, Junio C Hamano <gitster@pobox.com> wrote: >> David Aguilar <davvid@gmail.com> writes: >> >>> Do you have advice on how we should proceed? :sigh: sorry for wasting >>> so much maintainer time on this series already. If you need any >>> resends or anything please let me know. This time I'll wait for a >>> strong opinion before firing off patches. >>> >>> My opinion: yeah, v5's ([NO_]APPLE_COMMON_CRYPTO) was probably where >>> we should have stopped painting. Hindsight is 20/20. Luckily it >>> never left "pu". >> >> I could do this easily: >> >> $ git checkout da/darwin ;# b72ac20a6f73b >> $ git format-patch --stdout -2 | >> sed -e 's|COMMON_DIGEST_FOR_OPENSSL|APPLE_COMMON_CRYPTO|g' >P.mbox >> $ git checkout HEAD^^ ;# 29de20504e >> $ git am P.mbox >> $ git diff da/darwin HEAD ;# sanity check >> $ git log da/darwin.. ;# sanity check >> $ git branch -f da/darwin > > That would lose the one legitimate COMMON_DIGEST_FOR_OPENSSL which > needs to be defined before <CommonCrypto/CommonDigest.h> is included. It probably is a good catch, but I'll stop reading patches and start today's integration cycle (and will tag 1.8.3-rc3). At this point, I think the best would be for you to reroll these two patches, then after David reviews it and I'd re-queue the result with ... original commit message ... Reorginized use of Makefile variables and C preprosessor macros with help by Eric Sunshine. Signed-off-by: David Signed-off-by: Eric Signed-off-by: Me or something. Can you two help that? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-17 17:57 ` Junio C Hamano @ 2013-05-18 0:38 ` David Aguilar 2013-05-19 6:26 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: David Aguilar @ 2013-05-18 0:38 UTC (permalink / raw) To: Junio C Hamano Cc: Eric Sunshine, Torsten Bögershausen, Git List, Jonathan Nieder On Fri, May 17, 2013 at 10:57 AM, Junio C Hamano <gitster@pobox.com> wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > >> On Fri, May 17, 2013 at 12:53 PM, Junio C Hamano <gitster@pobox.com> wrote: >>> David Aguilar <davvid@gmail.com> writes: >>> >>>> Do you have advice on how we should proceed? :sigh: sorry for wasting >>>> so much maintainer time on this series already. If you need any >>>> resends or anything please let me know. This time I'll wait for a >>>> strong opinion before firing off patches. >>>> >>>> My opinion: yeah, v5's ([NO_]APPLE_COMMON_CRYPTO) was probably where >>>> we should have stopped painting. Hindsight is 20/20. Luckily it >>>> never left "pu". >>> >>> I could do this easily: >>> >>> $ git checkout da/darwin ;# b72ac20a6f73b >>> $ git format-patch --stdout -2 | >>> sed -e 's|COMMON_DIGEST_FOR_OPENSSL|APPLE_COMMON_CRYPTO|g' >P.mbox >>> $ git checkout HEAD^^ ;# 29de20504e >>> $ git am P.mbox >>> $ git diff da/darwin HEAD ;# sanity check >>> $ git log da/darwin.. ;# sanity check >>> $ git branch -f da/darwin >> >> That would lose the one legitimate COMMON_DIGEST_FOR_OPENSSL which >> needs to be defined before <CommonCrypto/CommonDigest.h> is included. > > It probably is a good catch, but I'll stop reading patches and start > today's integration cycle (and will tag 1.8.3-rc3). > > At this point, I think the best would be for you to reroll these two > patches, then after David reviews it and I'd re-queue the result > with > > ... original commit message ... > > Reorginized use of Makefile variables and C preprosessor > macros with help by Eric Sunshine. > > Signed-off-by: David > Signed-off-by: Eric > Signed-off-by: Me > > or something. Can you two help that? Thanks Eric and Junio. I looked over the patches and they look good. Signed-off-by: David Aguilar <davvid@gmail.com> Cheers, -- David ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-18 0:38 ` David Aguilar @ 2013-05-19 6:26 ` Junio C Hamano 2013-05-19 21:51 ` David Aguilar 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2013-05-19 6:26 UTC (permalink / raw) To: David Aguilar Cc: Eric Sunshine, Torsten Bögershausen, Git List, Jonathan Nieder David Aguilar <davvid@gmail.com> writes: > Thanks Eric and Junio. I looked over the patches and they look good. Are you sure about that? It seemed to me that it was breaking everybody that is not on MacOS X --- did I misread the patch? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-19 6:26 ` Junio C Hamano @ 2013-05-19 21:51 ` David Aguilar 0 siblings, 0 replies; 13+ messages in thread From: David Aguilar @ 2013-05-19 21:51 UTC (permalink / raw) To: Junio C Hamano Cc: Eric Sunshine, Torsten Bögershausen, Git List, Jonathan Nieder On Sat, May 18, 2013 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote: > David Aguilar <davvid@gmail.com> writes: > >> Thanks Eric and Junio. I looked over the patches and they look good. > > Are you sure about that? It seemed to me that it was breaking > everybody that is not on MacOS X --- did I misread the patch? Gah, correct. I've now tested v8 which Eric just sent out. It worked fine for me, with and without NO_APPLE_COMMON_CRYPTO. Thanks, -- David ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X 2013-05-17 8:21 ` David Aguilar 2013-05-17 16:53 ` Junio C Hamano @ 2013-05-17 17:40 ` Eric Sunshine 1 sibling, 0 replies; 13+ messages in thread From: Eric Sunshine @ 2013-05-17 17:40 UTC (permalink / raw) To: David Aguilar Cc: Torsten Bögershausen, Junio C Hamano, Git List, Jonathan Nieder On Fri, May 17, 2013 at 4:21 AM, David Aguilar <davvid@gmail.com> wrote: > On Thu, May 16, 2013 at 11:18 PM, Eric Sunshine <sunshine@sunshineco.com> wrote: >> On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen <tboegi@web.de> wrote: >>> On 2013-05-15 09.11, David Aguilar wrote: >>>> + 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-05-19 21:51 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-15 7:11 [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X David Aguilar 2013-05-15 7:11 ` [PATCH v5 2/2] imap-send: eliminate HMAC " David Aguilar 2013-05-15 17:56 ` [PATCH v5 1/2] cache.h: eliminate SHA-1 " Torsten Bögershausen 2013-05-17 6:18 ` Eric Sunshine 2013-05-17 8:21 ` David Aguilar 2013-05-17 16:53 ` Junio C Hamano 2013-05-17 17:20 ` David Aguilar 2013-05-17 17:29 ` Eric Sunshine 2013-05-17 17:57 ` Junio C Hamano 2013-05-18 0:38 ` David Aguilar 2013-05-19 6:26 ` Junio C Hamano 2013-05-19 21:51 ` David Aguilar 2013-05-17 17:40 ` Eric Sunshine
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).