httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Geoff Thorpe <ge...@geoffthorpe.net>
Subject Re: [PATCH] openssl configuration (v2)
Date Thu, 06 Mar 2003 22:16:56 GMT
Hi Justin,

* Justin Erenkrantz (justin@erenkrantz.com) wrote:
> 
> I think you mean adding -I<...> to CPPFLAGS not to CFLAGS.  That should be 
> portable and supported everywhere.  It's a C preprocessor flag not a flag 
> for the compiler.  Autoconf will evaluate ac_compile which includes 
> $CPPFLAGS. You should be able to use APR_ADDTO on CPPFLAGS for the explicit 
> path case. (Since the user specified it, we always need to add it to 
> CPPFLAGS.)

I was just getting ready to launch into a spiel about how none of the
documented flags seemed to be honoured by AC_CHECK_HEADERS and how I'd
looked at the generated configure script to see that cpp invocation
looked very different to compiler invocation, blah blah blah. Anyway, I
will spare you that spiel because having double checked I see that
you're absolutely right and I'm now wondering how I ended up so derailed
in the first place. I guess something else must have been broken whilst
I was trying that out and I never twigged. Anyway yes, putting the
include paths into CPPFLAGS makes header checking work. Thanks :-)

[snip]
> >  Apache's builtin tests (which are obviously OK because Apache links
> >  fine) should occur before the AC_CHECK_LIB()s for "ssl" and "crypto".
> >  See "Step 3" of my changes to acinclude.m4.
[snip]
> 
> A much easier route would be to include `$apr_config --libs` in that 
> section. Those libraries would be in there.  When we go to actually link, 
> the libtool dependencies will have them there.  But, you could add those to 
> LIBS temporarily - just remove them when you are done trying to link.

Well, this is exactly what I was looking for. Unfortunately it seems not
to work as-is. What happens is that if I remove my explicit
AC_CHECK_LIB() calls for dl, socket, nsl, etc and do the following;

   ...
   saved_LIBS=$LIBS
   LIBS="$LIBS `$apr_config --libs`"
   AC_CHECK_LIB(crypto, SSLeay_version)
   AC_CHECK_LIB(ssl, SSL_CTX_new)
   LIBS=$saved_LIBS
   ...

then strangely the -lcrypto and -lssl checks report success *yet* they
do not find their way into the linker flags (so apache fails to link
after compiling). I can only imagine 2 reasons for this; either my
restoring of LIBS to its original form obliterates the result of the
AC_CHECK_LIB() macros, or the fact the linker flags that -lcrypto and
-lssl depend have disappeared (and didn't occur in AC_CHECK_LIB() form)
mean that autoconf does away with -lcrypto and -lssl some point later. I
suspect the former as the latter seems just too weird a possibility.

> ap_platform_runtime_link_flag is '-R' on those platforms that need a 
> special flag to indicate where to look at run-time for libraries.  (Solaris 
> is the predominate case.)  Some platforms use '-rpath' as well.  -L is not 
> enough.

I figured it must be something of that sort from the existing code's
comments, thanks for clarifying. I mainly wanted to be sure it wasn't
producing something to compensate for the fact autoconf's own mechanisms
weren't being used. Can I assume then that I'm right to parrot the
existing code's production of the additional LDFLAGS entry if that flag
is set?

> >- I'm tagging "-DHAVE_OPENSSL" or "-DHAVE_SSLC" directly onto CFLAGS
> >  rather than using anything like AC_DEFINE because the latter
> >  possibility would require HAVE_OPENSSL and HAVE_SSLC to be stubbed
> >  into an appropriate "something.h.in" file. If you prefer not to have
> >  such stuff polluting CFLAGS then please suggest an appropriate ".in"
> >  file for me to hook into.
> 
> Why is having HAVE_OPENSSL or HAVE_SSLC in ap_config_auto.h a bad thing?  I 
> don't understand.

I didn't say it was a bad thing, on the contrary - I think it's a good
thing, I just wasn't sure which (if any) .in file would have been the
appropriate one. It's a big-ass tree of code that I have only very
selective familiarity with. :-) Anyway, following your comments, I
noticed that declaring the symbols in the top-level acconfig.h and
running "./buildconf" eventually propogating those values into
ap_config_auto.h.in and everything works. I'm happy with that, is it
what you had in mind?

> >-AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT,[
> >+AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT_OLD,[
> 
> Just toss the old APACHE_CHECK_SSL_TOOLKIT.  No need to keep the old 
> version around.

Yeah, but I'm leaving it like this while hashing out the patch details
because it makes the diff readable (if you remove the old version and
produce a diff, you'll see what I mean).

> >+    AC_MSG_CHECKING(for OpenSSL version)
> >+    AC_TRY_COMPILE([#include <openssl/opensslv.h>],
> >+[#if !defined(OPENSSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < 
> >0x0090609f
> >+#error "invalid openssl version"
> >+#endif],
> >+      [dnl Replace this with OPENSSL_VERSION_TEXT from opensslv.h?
> >+      AC_MSG_RESULT(OK)],
> 
> Yes, it should indicate the version found somehow.  I believe we did that 
> before.  I think it's worthwhile to have.

I'm all for it, but would rather not guess as to the preferred mechanism
for slurping that text out of the header file (especially as we're not
supposed to assume anything about its location). I imagine the proper
way is to have a compile test spit the string out and then capture it
somehow. No doubt an autoconf-guru knows the relevant macro trick, but
on my own I'm more likely to wildly overcomplicate this code and bring
in portability problems. Unless someone tells me the Apache-Approved(tm)
way to do this, I'd rather just leave the comment there to guide someone
else if they feel so moved afterwards.

> > dnl #  hook module into the Autoconf mechanism (--enable-ssl option)
> > APACHE_MODULE(ssl, [SSL/TLS support (mod_ssl)], $ssl_objs, , no, [
> >     APACHE_CHECK_SSL_TOOLKIT
> >-    AC_CHECK_FUNCS(SSL_set_state)
> >-    AC_CHECK_FUNCS(SSL_set_cert_store)
> >+    dnl These checks aren't really useful and could fail for silly 
> >reasons 
> 
> Just remove them altogether.

Agreed.

> The rest of it looks good.  -- justin

Thanks for the feedback. I've incorporated all the above and attached a
new patch. Does this seem OK? There are a couple of notes I want to
leave you with;

- I've incorporated the use of `$apr_config --libs` as you suggested and
  so my patch is currently broken, but I want to head in the right
  direction. Any ideas why the successfully reported -lssl -lcrypto
  flags disappear from the generated Makefile's?

- I've moved the version checks into the header check steps so I don't
  have to save and restore CPPFLAGS twice.

- Seeing as CPPFLAGS seems ideal for header *and* compilation checks, is
  INCLUDES still the appropriate place to APR_ADDTO() any required
  include path once the tests are done?

Cheers,
Geoff

-- 
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/


Mime
View raw message