patch, 3 partsMakefile: Fix APPLE_COMMON_CRYPTO with BLK_SHA1

> It used to be that APPLE_COMMON_CRYPTO did nothing when BLK_SHA1 was
> set.  But APPLE_COMMON_CRYPTO is now used for more than just SHA1 (see
> 3ef2bca) so make sure that the appropriate libraries are always set.
> 
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> ---
> Makefile | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 82f2e22..7051956 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1182,6 +1182,9 @@ ifdef NEEDS_SSL_WITH_CRYPTO
> else
> 	LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto
> endif
> +ifdef APPLE_COMMON_CRYPTO
> +	LIB_4_CRYPTO += -framework Security -framework CoreFoundation
> +endif
> endif
> ifdef NEEDS_LIBICONV
> 	ifdef ICONVDIR
> @@ -1413,7 +1416,6 @@ ifdef PPC_SHA1
> 	LIB_H += ppc/sha1.h
> else
> ifdef APPLE_COMMON_CRYPTO
> -	LIB_4_CRYPTO += -framework Security -framework CoreFoundation
> 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
> 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
> else
> -- 
> 1.8.4.rc1.384.g0976a17.dirty
> 

> It used to be that APPLE_COMMON_CRYPTO did nothing when BLK_SHA1 was
> set.  But APPLE_COMMON_CRYPTO is now used for more than just SHA1 (see
> 3ef2bca) so make sure that the appropriate libraries are always set.
>
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> ---
>  Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 82f2e22..7051956 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1182,6 +1182,9 @@ ifdef NEEDS_SSL_WITH_CRYPTO
>  else
>  	LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto
>  endif
> +ifdef APPLE_COMMON_CRYPTO
> +	LIB_4_CRYPTO += -framework Security -framework CoreFoundation
> +endif
>  endif
>  ifdef NEEDS_LIBICONV
>  	ifdef ICONVDIR
> @@ -1413,7 +1416,6 @@ ifdef PPC_SHA1
>  	LIB_H += ppc/sha1.h
>  else
>  ifdef APPLE_COMMON_CRYPTO
> -	LIB_4_CRYPTO += -framework Security -framework CoreFoundation
>  	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>  	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
>  else

> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
>
>> It used to be that APPLE_COMMON_CRYPTO did nothing when BLK_SHA1 was
>> set.  But APPLE_COMMON_CRYPTO is now used for more than just SHA1 (see
>> 3ef2bca) so make sure that the appropriate libraries are always set.
>>
>> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
>> ---
>>  Makefile | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 82f2e22..7051956 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1182,6 +1182,9 @@ ifdef NEEDS_SSL_WITH_CRYPTO
>>  else
>>       LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto
>>  endif
>> +ifdef APPLE_COMMON_CRYPTO
>> +     LIB_4_CRYPTO += -framework Security -framework CoreFoundation
>> +endif
>>  endif
>>  ifdef NEEDS_LIBICONV
>>       ifdef ICONVDIR
>> @@ -1413,7 +1416,6 @@ ifdef PPC_SHA1
>>       LIB_H += ppc/sha1.h
>>  else
>>  ifdef APPLE_COMMON_CRYPTO
>> -     LIB_4_CRYPTO += -framework Security -framework CoreFoundation
>>       COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>>       SHA1_HEADER = <CommonCrypto/CommonDigest.h>
>>  else
>
> Hmph.
>
> So the people previously tested this must have built imap-send
> without blk-sha1, which not just linked with these libs but also
> included the <CommonCrypto/CommonDigest.h> header file and defined
> the -DCOMMON_DIGEST_FOR_OPENSSL preprocessor macro.  Building with
> blk-sha1 would not have worked for them.
> Now we always link with these libraries, even when building with
> blk-sha1.  Do the COMPAT_CFLAGS and SHA1_HEADER pieces only needed
> when using the SHA1 digest implementation from CommonCrypto and
> nothing imap-send uses?

> compat/apple-common-crypto.h uses die() in one of its macros, but was
> included in git-compat-util.h before the definition of die.
> 
> Fix by simply moving the relevant block after the die/error/warning
> declarations.
> 
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> ---
> 
> Not sure if this is the best place to move it to, but it's the earliest it can
> be in the file without causing errors.  (Namely that clang has to guess what
> die() means in apple-common-crypto.h and guesses differently than the actual
> definition.)
> 
> git-compat-util.h | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index af5f6bb..d60e28d 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -129,16 +129,6 @@
> #include <poll.h>
> #endif
> 
> -#ifndef NO_OPENSSL
> -#ifdef APPLE_COMMON_CRYPTO
> -#include "compat/apple-common-crypto.h"
> -#else
> -#include <openssl/evp.h>
> -#include <openssl/hmac.h>
> -#endif /* APPLE_COMMON_CRYPTO */
> -#include <openssl/x509v3.h>
> -#endif /* NO_OPENSSL */
> -
> #if defined(__MINGW32__)
> /* pull in Windows compatibility stuff */
> #include "compat/mingw.h"
> @@ -340,6 +330,16 @@ extern NORETURN void die_errno(const char *err, ...) __attribute__((format (prin
> extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
> extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
> 
> +#ifndef NO_OPENSSL
> +#ifdef APPLE_COMMON_CRYPTO
> +#include "compat/apple-common-crypto.h"
> +#else
> +#include <openssl/evp.h>
> +#include <openssl/hmac.h>
> +#endif /* APPLE_COMMON_CRYPTO */
> +#include <openssl/x509v3.h>
> +#endif /* NO_OPENSSL */
> +
> /*
>  * Let callers be aware of the constant return value; this can help
>  * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though,
> -- 
> 1.8.4.rc1.384.g0976a17.dirty
> 

> compat/apple-common-crypto.h uses die() in one of its macros, but was
> included in git-compat-util.h before the definition of die.
>
> Fix by simply moving the relevant block after the die/error/warning
> declarations.

> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> ---
>
>  Not sure if this is the best place to move it to, but it's the earliest it can
>  be in the file without causing errors.  (Namely that clang has to guess what
>  die() means in apple-common-crypto.h and guesses differently than the actual
>  definition.)
>
>  git-compat-util.h | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index af5f6bb..d60e28d 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -129,16 +129,6 @@
>  #include <poll.h>
>  #endif
>  
> -#ifndef NO_OPENSSL
> -#ifdef APPLE_COMMON_CRYPTO
> -#include "compat/apple-common-crypto.h"
> -#else
> -#include <openssl/evp.h>
> -#include <openssl/hmac.h>
> -#endif /* APPLE_COMMON_CRYPTO */
> -#include <openssl/x509v3.h>
> -#endif /* NO_OPENSSL */
> -
>  #if defined(__MINGW32__)
>  /* pull in Windows compatibility stuff */
>  #include "compat/mingw.h"
> @@ -340,6 +330,16 @@ extern NORETURN void die_errno(const char *err, ...) __attribute__((format (prin
>  extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  
> +#ifndef NO_OPENSSL
> +#ifdef APPLE_COMMON_CRYPTO
> +#include "compat/apple-common-crypto.h"
> +#else
> +#include <openssl/evp.h>
> +#include <openssl/hmac.h>
> +#endif /* APPLE_COMMON_CRYPTO */
> +#include <openssl/x509v3.h>
> +#endif /* NO_OPENSSL */
> +
>  /*
>   * Let callers be aware of the constant return value; this can help
>   * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though,

> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
>
>> compat/apple-common-crypto.h uses die() in one of its macros, but was
>> included in git-compat-util.h before the definition of die.
>>
>> Fix by simply moving the relevant block after the die/error/warning
>> declarations.
>
> Puzzled.  What needs fixing???
>
> Ahh, that one is not just making #define macros, but defining static
> inline functions.
>
> I wonder if they need to be static inlines to be duplicated at each
> call sites in the first place.  Wouldn't it be better to create a
> compat/something.c file to be linked with?

>
> The URL included in the header appears to vary from curl version to
> curl version.  Since we only care about the final few lines, only test
> them.  However, make sure the blank line after the header is still
> included to make sure there are no extra cookie lines.
>
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> ---
>
>  I suppose a sed invocation to strip out the URL or comments might be better,
>  but this seemed simpler.
>
>  t/t5551-http-fetch.sh | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
> index 287d22b..8196af1 100755
> --- a/t/t5551-http-fetch.sh
> +++ b/t/t5551-http-fetch.sh
> @@ -191,9 +191,6 @@ cat >cookies.txt <<EOF
>  127.0.0.1      FALSE   /smart_cookies/ FALSE   0       othername       othervalue
>  EOF
>  cat >expect_cookies.txt <<EOF
> -# Netscape HTTP Cookie File
> -# http://curl.haxx.se/docs/http-cookies.html
> -# This file was generated by libcurl! Edit at your own risk.
>
>  127.0.0.1      FALSE   /smart_cookies/ FALSE   0       othername       othervalue
>  127.0.0.1      FALSE   /smart_cookies/repo.git/info/   FALSE   0       name    value
> @@ -202,7 +199,8 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set
>         git config http.cookiefile cookies.txt &&
>         git config http.savecookies true &&
>         git ls-remote $HTTPD_URL/smart_cookies/repo.git master &&
> -       test_cmp expect_cookies.txt cookies.txt
> +       tail -3 cookies.txt > cookies_tail.txt

>
> +       test_cmp expect_cookies.txt cookies_tail.txt
>  '
>
>  test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
> --
> 1.8.4.rc1.384.g0976a17.dirty
>