* [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
@ 2009-09-08 13:54 Brian Gernhardt
2009-09-08 20:19 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Brian Gernhardt @ 2009-09-08 13:54 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
The Makefile comment for NEEDS_SSL_WITH_CRYPTO says to define it "if
you need -lcrypto with -lssl (Darwin)." However, what it actually
does is add -lssl when you use -lcrypto and not the other way around.
However, libcrypto contains a majority of the ERR_* functions from
OpenSSL (at least on OS X) so we need it both ways.
So, add NEEDS_CRYPTO_WITH_SSL which adds -lcrypto to the OpenSSL link
flags and clarify the difference between it and NEEDS_SSL_WITH_CRYPTO.
Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---
Compilation using BLK_SHA1 on OS X 10.5 and 10.6 (at least) is still
broken without this patch.
Alex Riesen <raa.lkml@gmail.com> pointed out that just adding LIB_4_CRYPTO
to git-imap-send is simpler, but judging from the fact that nobody else
has complained about this issue, I'm guessing that the need for -lcrypto
when using -lssl is not widespread. (Or BLK_SHA1 isn't getting used much
or those who do don't compile git-imap-send with SSL.)
Makefile | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index ce882d0..121be04 100644
--- a/Makefile
+++ b/Makefile
@@ -91,7 +91,9 @@ all::
# Define PPC_SHA1 environment variable when running make to make use of
# a bundled SHA1 routine optimized for PowerPC.
#
-# Define NEEDS_SSL_WITH_CRYPTO if you need -lcrypto with -lssl (Darwin).
+# Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
+#
+# Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
#
# Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).
#
@@ -707,6 +709,7 @@ ifeq ($(uname_S),SCO_SV)
TAR = gtar
endif
ifeq ($(uname_S),Darwin)
+ NEEDS_CRYPTO_WITH_SSL = YesPlease
NEEDS_SSL_WITH_CRYPTO = YesPlease
NEEDS_LIBICONV = YesPlease
ifeq ($(shell expr "$(uname_R)" : '[15678]\.'),2)
@@ -1009,6 +1012,9 @@ ifndef NO_OPENSSL
else
OPENSSL_LINK =
endif
+ ifdef NEEDS_CRYPTO_WITH_SSL
+ OPENSSL_LINK += -lcrypto
+ endif
else
BASIC_CFLAGS += -DNO_OPENSSL
BLK_SHA1 = 1
--
1.6.4.2.420.g30ecf
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
2009-09-08 13:54 [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL Brian Gernhardt
@ 2009-09-08 20:19 ` Junio C Hamano
2009-09-09 0:40 ` Brian Gernhardt
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2009-09-08 20:19 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git List
Brian Gernhardt <brian@gernhardtsoftware.com> writes:
> The Makefile comment for NEEDS_SSL_WITH_CRYPTO says to define it "if
> you need -lcrypto with -lssl (Darwin)." However, what it actually
> does is add -lssl when you use -lcrypto and not the other way around.
Correct. That is worth fixing.
> However, libcrypto contains a majority of the ERR_* functions from
> OpenSSL (at least on OS X) so we need it both ways.
I see. We need to be able to see ERR_err_string(), whose caller happens
to be only imap-send.c for now, and that function is in -lcrypto together
with other ERR_* functions.
> Compilation using BLK_SHA1 on OS X 10.5 and 10.6 (at least) is still
> broken without this patch.
>
> Alex Riesen <raa.lkml@gmail.com> pointed out that just adding LIB_4_CRYPTO
> to git-imap-send is simpler, but judging from the fact that nobody else
> has complained about this issue, I'm guessing that the need for -lcrypto
> when using -lssl is not widespread. (Or BLK_SHA1 isn't getting used much
> or those who do don't compile git-imap-send with SSL.)
BLK_SHA1 is fairly new on 'master', so lack of breakage report is
expected.
The patch makes sense to me, but as the result, depending on platforms
and configuration, we would use three variations when linking imap-send
with no NO_OPENSSL defined:
* Both -lcrypto -lssl
* Only -lssl
* Only -lcrypto
I wonder if we can simplify this in some way (not a 1.6.5 topic).
I am suspecting that the reason we do not say "always both" is because
depending on the vintage of OpenSSL one or the other is missing?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
2009-09-08 20:19 ` Junio C Hamano
@ 2009-09-09 0:40 ` Brian Gernhardt
0 siblings, 0 replies; 6+ messages in thread
From: Brian Gernhardt @ 2009-09-09 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
On Sep 8, 2009, at 4:19 PM, Junio C Hamano wrote:
> The patch makes sense to me, but as the result, depending on platforms
> and configuration, we would use three variations when linking imap-
> send
> with no NO_OPENSSL defined:
>
> * Both -lcrypto -lssl
> * Only -lssl
> * Only -lcrypto
>
> I wonder if we can simplify this in some way (not a 1.6.5 topic).
>
> I am suspecting that the reason we do not say "always both" is because
> depending on the vintage of OpenSSL one or the other is missing?
It just occurred to me that I have a Debian machine to test this on.
I generally don't use my web server for testing, but testing a compile
should be safe enough.
Sure enough, it compiles without this patch. It may be a difference
between the linkers. It appears that ERR_error_string() is in
libcrypto for both OS X and Debian and that libssl.{so,dylib} both
have load commands for libcrypto.{so,dylib}, but OS X ld is pickier
than Debian ld. Odd.
I don't see a scenario where only -lcrypto would be used for imap-
send, actually. That would imply that we're using OpenSSL's SHA-1
algorithm but not using it for IMAPS access. SSL_WITH_CRYPTO is about
needing -lssl when using -lcrypto for SHA-1 while CRYPTO_WITH_SSL is
about needing -lcrypto when using -lssl for SSL.
The only simplification I can think of is to make
NEEDS_CRYPTO_WITH_SSL and NEEDS_SSL_WITH_CRYPTO be the same, so that
it will include both any time it includes either one.
NEEDS_SSL_WITH_CRYPTO is set for UnixWare, SCO_SV, and OS X. I only
have the latter to test with so I don't know if the symmetry is true
for the other two.
~~ Brian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
2009-08-24 9:45 ` Alex Riesen
@ 2009-08-24 14:22 ` Brian Gernhardt
0 siblings, 0 replies; 6+ messages in thread
From: Brian Gernhardt @ 2009-08-24 14:22 UTC (permalink / raw)
To: Alex Riesen; +Cc: Git List, Junio C Hamano
On Aug 24, 2009, at 5:45 AM, Alex Riesen wrote:
> As imap-send is the only one which uses the symbols, why not just add
> LIB_4_CRYPTO
> to its linking command? Like in the broken GMail-patch below:
I didn't do this because I didn't know if all platforms needed
libcrypto to compile imap-send. If it is, then this is obviously the
simpler solution.
> diff --git a/Makefile b/Makefile
> index 02ff867..33971f3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1503,7 +1503,7 @@ git-%$X: %.o $(GITLIBS)
>
> git-imap-send$X: imap-send.o $(GITLIBS)
> $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,
> $^) \
> - $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
> + $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
>
> http.o http-walker.o http-push.o: http.h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
2009-08-15 16:46 Brian Gernhardt
@ 2009-08-24 9:45 ` Alex Riesen
2009-08-24 14:22 ` Brian Gernhardt
0 siblings, 1 reply; 6+ messages in thread
From: Alex Riesen @ 2009-08-24 9:45 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git List, Junio C Hamano
On Sat, Aug 15, 2009 at 18:46, Brian
Gernhardt<brian@gernhardtsoftware.com> wrote:
>
> After adding BLK_SHA1 to my config.mak, git-imap-send started giving me link
> errors:
>
> Undefined symbols:
> "_ERR_get_error", referenced from:
> _ssl_socket_perror in imap-send.o
> "_ERR_error_string", referenced from:
> _ssl_socket_perror in imap-send.o
>
> Some investigation led me to the fact that BLK_SHA1 removes LIB_4_CRYPTO from
> EXTLIBS. That let me find the missing functions in libcrypto. At first I
> considered making NEEDS_SSL_WITH_CRYPTO add -lcrypto to the SSL build flags
> but decided to go this route in case there are platforms that need it one way
> around and not the other.
As imap-send is the only one which uses the symbols, why not just add
LIB_4_CRYPTO
to its linking command? Like in the broken GMail-patch below:
diff --git a/Makefile b/Makefile
index 02ff867..33971f3 100644
--- a/Makefile
+++ b/Makefile
@@ -1503,7 +1503,7 @@ git-%$X: %.o $(GITLIBS)
git-imap-send$X: imap-send.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
+ $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
http.o http-walker.o http-push.o: http.h
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
@ 2009-08-15 16:46 Brian Gernhardt
2009-08-24 9:45 ` Alex Riesen
0 siblings, 1 reply; 6+ messages in thread
From: Brian Gernhardt @ 2009-08-15 16:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
The Makefile comment for NEEDS_SSL_WITH_CRYPTO says to define it "if
you need -lcrypto with -lssl (Darwin)." However, what it actually
does is add -lssl when you use -lcrypto and not the other way around.
However, libcrypto contains a majority of the ERR_* functions from
OpenSSL (at least on OS X) so we need it both ways.
So, add NEEDS_CRYPTO_WITH_SSL which adds -lcrypto to the OpenSSL link
flags and clarify the difference between it and NEEDS_SSL_WITH_CRYPTO.
---
After adding BLK_SHA1 to my config.mak, git-imap-send started giving me link
errors:
Undefined symbols:
"_ERR_get_error", referenced from:
_ssl_socket_perror in imap-send.o
"_ERR_error_string", referenced from:
_ssl_socket_perror in imap-send.o
Some investigation led me to the fact that BLK_SHA1 removes LIB_4_CRYPTO from
EXTLIBS. That let me find the missing functions in libcrypto. At first I
considered making NEEDS_SSL_WITH_CRYPTO add -lcrypto to the SSL build flags
but decided to go this route in case there are platforms that need it one way
around and not the other.
I've enabled this build option by default on Darwin but nowhere else. If you
can't build git-imap-send (with SSL) after enabling BLK_SHA1, your platform
may need this flag as well.
Makefile | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index f94fe05..bc745fb 100644
--- a/Makefile
+++ b/Makefile
@@ -99,7 +99,9 @@ all::
# on non-x86 architectures (e.g. PowerPC), while the OpenSSL version (default
# choice) has very fast version optimized for i586.
#
-# Define NEEDS_SSL_WITH_CRYPTO if you need -lcrypto with -lssl (Darwin).
+# Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
+#
+# Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
#
# Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).
#
@@ -714,6 +716,7 @@ ifeq ($(uname_S),SCO_SV)
TAR = gtar
endif
ifeq ($(uname_S),Darwin)
+ NEEDS_CRYPTO_WITH_SSL = YesPlease
NEEDS_SSL_WITH_CRYPTO = YesPlease
NEEDS_LIBICONV = YesPlease
ifeq ($(shell expr "$(uname_R)" : '[15678]\.'),2)
@@ -1023,6 +1026,9 @@ ifndef NO_OPENSSL
else
OPENSSL_LINK =
endif
+ ifdef NEEDS_CRYPTO_WITH_SSL
+ OPENSSL_LINK += -lcrypto
+ endif
else
BASIC_CFLAGS += -DNO_OPENSSL
MOZILLA_SHA1 = 1
--
1.6.4.244.ge5cd0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-09-09 0:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-08 13:54 [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL Brian Gernhardt
2009-09-08 20:19 ` Junio C Hamano
2009-09-09 0:40 ` Brian Gernhardt
-- strict thread matches above, loose matches on Subject: below --
2009-08-15 16:46 Brian Gernhardt
2009-08-24 9:45 ` Alex Riesen
2009-08-24 14:22 ` Brian Gernhardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.