You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Justin Erenkrantz <ju...@erenkrantz.com> on 2004/08/01 00:37:24 UTC
Re: 1.0.0 RC5
--On Friday, July 30, 2004 8:33 AM -0400 Ryan Bloom <rb...@gmail.com> wrote:
> However, we need to remove the APR_STATUS_IS_SUCCESS macro before 1.0 goes
> out, because otherwise we are stuck with it for a very long time.
It's gone now. ;-)
I've got to fix up httpd 2.1 now and then I'll review Max's patch. -- justin
Re: 1.0.0 RC5
Posted by Graham Leggett <mi...@sharp.fm>.
William A. Rowe, Jr. wrote:
>>The two most important bits of the LDAP support are ./configure magic to find the right toolkit, and the newly overhauled apr_ldap_init.c, which replaces the LDAP SDK's inconsistent ldap_init() function. These should work fine.
> No - the most important bit is to start hiding the details in apu_private.h
> and quit publicizing the sdk versions, define mapping wrappers, etc.
> Once everything that aprutil-1 elects to do is hidden inside aprutil-1.so,
> and the interface is always the same (no matter which linkage), then
> we have 1. defined binary compatibility, and 2. stuck to it :)
I was referring to the most important bit of the functionality - you're
referring to the most serious bit of the fooness :)
I think the code with the most fooness is also the code with the least
impact - so chopping it shouldn't cause too many problems, and certainly
not problems that can't be fixed in v1.0.1.
>>Then cvs rm apr_ldap_url.c, which seems to be the main issue. We can fix it and put it back in the next release.
> Actually, isn't that the most trivial to fix?
>
> #if HAVE_ldap_parse_url
> return ldap_parse_url(...);
> #else
> [all the code]
> #endif
What it does now is this:
#if !APR_HAS_LDAP_URL_PARSE
[all the code]
#endif
Inside the code it then defines apr_ldap_is_ldap_url() - but only if
APR_HAS_LDAP_URL_PARSE is false, which makes no sense.
This is definitely bogus - let me see if I can fix it.
>>What issues are there with apr_ldap_compat.c? This code is very short, and any issues are probably easily fixed.
> iirc this is a bunch of macros. Any code linked against one flavor
> of aprutil-1.so can't be expected to load under another. This is
> problematic, no?
Looking at this further, the apr_ldap_compat is a "fill in" function for
LDAP v2 servers. It defines ldap_search_ext_s() and ldap_memfree() for
LDAP v2 servers, where these functions were missing from the C SDK.
In the header file, the macro defines handling a difference between the
v2 LDAP SDK and the v3 one.
Therefore I ask: How important is LDAP v2? Can we just chop LDAP v2
support for APR 1.0 - we get rid of the macros and the spare functions.
We can do LDAP v2.0 support (as opposed to v3.0 support we do by
default) in the proper APR way later.
Regards,
Graham
--
Re: 1.0.0 RC5
Posted by Graham Leggett <mi...@sharp.fm>.
William A. Rowe, Jr. wrote:
> No - the most important bit is to start hiding the details in apu_private.h
> and quit publicizing the sdk versions, define mapping wrappers, etc.
> Once everything that aprutil-1 elects to do is hidden inside aprutil-1.so,
> and the interface is always the same (no matter which linkage), then
> we have 1. defined binary compatibility, and 2. stuck to it :)
apr_ldap_compat is gone, support for old LDAP SDK v2.0 has been dropped.
The macro that resolved to ldap_parse_url has been dropped and replaced
with code that parses URLs ourselves the same way on all platforms,
allocating memory from pools. All the *free methods have been dropped.
The httpd mod_auth_ldap and mod_ldap modules have been updated to use
the new functions, there are no more toolkit specific defines used
anywhere in httpd (unless I missed one).
If you could double check the stuff that is there now to confirm that
all the fooness is dead, I would be grateful.
Regards,
Graham
--
Re: 1.0.0 RC5
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 06:08 AM 8/3/2004, Graham Leggett wrote:
>William A. Rowe, Jr. wrote:
>
>> Right now it does little. Graham and I agree on the right solution,
>> to abstract out the logic to do SSL connections in a portable way.
>> There will be no need for the 'application developer' to know which
>> toolkit they are using. We will make that transparent.
>
>This has already been done, and is already checked into apr_util.
Very cool! I am traveling non-stop so haven't dug into the code, and
probably can't till Tuesday. That 40% of the 80%...
>>Least action is fork 1.1, cvs rm the offensive files, and change about
>>10 lines of the makefiles and rip out the ldap detection. Pretty trivial.
>
>That's overkill.
>
>The two most important bits of the LDAP support are ./configure magic to find the right toolkit, and the newly overhauled apr_ldap_init.c, which replaces the LDAP SDK's inconsistent ldap_init() function. These should work fine.
No - the most important bit is to start hiding the details in apu_private.h
and quit publicizing the sdk versions, define mapping wrappers, etc.
Once everything that aprutil-1 elects to do is hidden inside aprutil-1.so,
and the interface is always the same (no matter which linkage), then
we have 1. defined binary compatibility, and 2. stuck to it :)
>>Deprecating means we support this junk till 2.0 releases.
>
>Then cvs rm apr_ldap_url.c, which seems to be the main issue. We can fix it and put it back in the next release.
Actually, isn't that the most trivial to fix?
#if HAVE_ldap_parse_url
return ldap_parse_url(...);
#else
[all the code]
#endif
>What issues are there with apr_ldap_compat.c? This code is very short, and any issues are probably easily fixed.
iirc this is a bunch of macros. Any code linked against one flavor
of aprutil-1.so can't be expected to load under another. This is
problematic, no?
Bill
Re: 1.0.0 RC5
Posted by Graham Leggett <mi...@sharp.fm>.
William A. Rowe, Jr. wrote:
> Right now it does little. Graham and I agree on the right solution,
> to abstract out the logic to do SSL connections in a portable way.
> There will be no need for the 'application developer' to know which
> toolkit they are using. We will make that transparent.
This has already been done, and is already checked into apr_util.
> Least action is fork 1.1, cvs rm the offensive files, and change about
> 10 lines of the makefiles and rip out the ldap detection. Pretty trivial.
That's overkill.
The two most important bits of the LDAP support are ./configure magic to
find the right toolkit, and the newly overhauled apr_ldap_init.c, which
replaces the LDAP SDK's inconsistent ldap_init() function. These should
work fine.
> Deprecating means we support this junk till 2.0 releases.
Then cvs rm apr_ldap_url.c, which seems to be the main issue. We can fix
it and put it back in the next release.
What issues are there with apr_ldap_compat.c? This code is very short,
and any issues are probably easily fixed.
Regards,
Graham
--
Re: 1.0.0 RC5
Posted by Graham Leggett <mi...@sharp.fm>.
Justin Erenkrantz wrote:
> Under our compatibility rules, we can remove apr_ldap_* in APR 1.0 and
> add it back in APR 1.1. That's my recommendation here. -- justin
As I have just overhauled apr_ldap, I would ask that people check first
which parts of apr_ldap need to be removed and/or fixed.
I don't want to see code chopped out because a whole lot of different
code is broken. :(
Regards,
Graham
--
Re: 1.0.0 RC5
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Tuesday, August 3, 2004 12:16 AM -0400 Cliff Woolley
<jw...@virginia.edu> wrote:
> +1. If the API is really that broken, it's better to remove it
> completely and add something better later.
Under our compatibility rules, we can remove apr_ldap_* in APR 1.0 and add
it back in APR 1.1. That's my recommendation here. -- justin
Re: 1.0.0 RC5
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 2 Aug 2004, William A. Rowe, Jr. wrote:
> Least action is fork 1.1, cvs rm the offensive files, and change about
> 10 lines of the makefiles and rip out the ldap detection. Pretty trivial.
> Deprecating means we support this junk till 2.0 releases.
+1. If the API is really that broken, it's better to remove it
completely and add something better later.
Re: 1.0.0 RC5
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 12:53 PM 8/2/2004, David Reid wrote:
>Graham Leggett wrote:
>
>>William A. Rowe, Jr. wrote:
>>
>>>I would be +1 to simply remove auth_ldap from APR 1.0, and reintroduce
>>>the entire feature in the new APR 1.1 (which we can start development
>>>on immediately.) And that presumes httpd 2.1/2.2 will depend on the
>>>1.1 release of apr-util.
>>>I hate to hold up 1.0 any further. I would hate to release apr-util as-in
>>>even more. Thoughts or comments about my solution above?
>
>Why (I feel I have to ask) is this ONLY just coming to light now?
Well, it isn't. Several APR contributors have griped that apr-util would
become nothing more than a garbage pit of hacks-upon-hacks around
broken/inconsistent APIs. My only ldap hacks to date were on win32.
Now I'm deeply immersed in config and build issues for many more
unix platforms, and was hacking in Sun LDAP SDK support. It's when
I discovered apr-util's implementation of ldap was valueless for writing
portable code. If our code doesn't facilitate portable code, it doesn't
belong in APR.
So I threw it to the list, drop the existing implementation? I'm NOT
suggesting we put it back in before APR 1.0 release.
>How much work is needed to fix it? What exactly is broken about it?
Right now it does little. Graham and I agree on the right solution, to
abstract out the logic to do SSL connections in a portable way. There
will be no need for the 'application developer' to know which toolkit
they are using. We will make that transparent.
>>Ripping it out is a whole lot of work best avoided IMHO.
>
>Let's take the course of least action shall we?
Least action is fork 1.1, cvs rm the offensive files, and change about
10 lines of the makefiles and rip out the ldap detection. Pretty trivial.
Deprecating means we support this junk till 2.0 releases.
Bill
Re: 1.0.0 RC5
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 04:04 PM 8/2/2004, Graham Leggett wrote:
>David Reid wrote:
>
>>>The main fooness is in apr_ldap_url.c. Can we not mark this code as deprecated in v1.0, which should hopefully warn alert coders that the code should not be used, and can be pointed out to coders who are asleep otherwise if they moan.
>
>>How much work is needed to fix it? What exactly is broken about it?
>
>It needs an overhaul as it does some "wrong" things. Fixing it can wait till APR v1.1. In the meantime we can mark what's there as deprecated so as to not
>cause confusion.
If they need the broken implementation, they can keep using 0.9 - in the
meantime let's design the right interface by 1.1 and completely REMOVE,
not 'deprecate' the existing flavor.
Deprecation isn't getting rid of a feature until the next release. This IS
the next release, so just branch 1.1-dev, and remove the ldap files on the
1.0 branch, knock out the config.m4 magic, and we have a valid 1.0 release.
>>Let's take the course of least action shall we?
>>Is this release creating a record for being the longest in history?
>>I'm away for most of the rest of the week. I had hoped to have RC5 rolled today, but guess that isn't going to happen now. *sigh*
>
>I have added a note that the two offending LDAP source files (apr_ldap_compat and apr_ldap_url) are deprecated subject to an overhaul.
>
>Quick - roll RC5!
I'll vote -1 on this sheist - however that's just a vote. Shall see if anyone
else is this disgusted.
David, if we are at 1.0, shall we branch 1.1 right now and I'll do the dirty
deed myself tomorrow night at the hotel?
Bill
Re: 1.0.0 RC5
Posted by Graham Leggett <mi...@sharp.fm>.
David Reid wrote:
>> The main fooness is in apr_ldap_url.c. Can we not mark this code as
>> deprecated in v1.0, which should hopefully warn alert coders that the
>> code should not be used, and can be pointed out to coders who are
>> asleep otherwise if they moan.
> How much work is needed to fix it? What exactly is broken about it?
It needs an overhaul as it does some "wrong" things. Fixing it can wait
till APR v1.1. In the meantime we can mark what's there as deprecated so
as to not cause confusion.
> Let's take the course of least action shall we?
>
> Is this release creating a record for being the longest in history?
>
> I'm away for most of the rest of the week. I had hoped to have RC5
> rolled today, but guess that isn't going to happen now. *sigh*
I have added a note that the two offending LDAP source files
(apr_ldap_compat and apr_ldap_url) are deprecated subject to an overhaul.
Quick - roll RC5!
Regards,
Graham
--
Re: 1.0.0 RC5
Posted by David Reid <da...@jetnet.co.uk>.
Graham Leggett wrote:
> William A. Rowe, Jr. wrote:
>
>> I would be +1 to simply remove auth_ldap from APR 1.0, and reintroduce
>> the entire feature in the new APR 1.1 (which we can start development
>> on immediately.) And that presumes httpd 2.1/2.2 will depend on the
>> 1.1 release of apr-util.
>> I hate to hold up 1.0 any further. I would hate to release apr-util
>> as-in
>> even more. Thoughts or comments about my solution above?
>
Why (I feel I have to ask) is this ONLY just coming to light now?
>
> The main fooness is in apr_ldap_url.c. Can we not mark this code as
> deprecated in v1.0, which should hopefully warn alert coders that the
> code should not be used, and can be pointed out to coders who are asleep
> otherwise if they moan.
How much work is needed to fix it? What exactly is broken about it?
> Ripping it out is a whole lot of work best avoided IMHO.
Let's take the course of least action shall we?
Is this release creating a record for being the longest in history?
I'm away for most of the rest of the week. I had hoped to have RC5
rolled today, but guess that isn't going to happen now. *sigh*
david
Re: 1.0.0 RC5
Posted by Graham Leggett <mi...@sharp.fm>.
William A. Rowe, Jr. wrote:
> I would be +1 to simply remove auth_ldap from APR 1.0, and reintroduce
> the entire feature in the new APR 1.1 (which we can start development
> on immediately.) And that presumes httpd 2.1/2.2 will depend on the
> 1.1 release of apr-util.
>
> I hate to hold up 1.0 any further. I would hate to release apr-util as-in
> even more. Thoughts or comments about my solution above?
The main fooness is in apr_ldap_url.c. Can we not mark this code as
deprecated in v1.0, which should hopefully warn alert coders that the
code should not be used, and can be pointed out to coders who are asleep
otherwise if they moan.
Ripping it out is a whole lot of work best avoided IMHO.
Regards,
Graham
--
Re: 1.0.0 RC5
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 12:11 PM 8/1/2004, Graham Leggett wrote:
>David Reid wrote:
>
>>So I see. I'll tag & roll APR RC5 later on tonight and hopefully as soon as apr-util is patched for apr-config I'll be able to roll.
>
>Would it be possible to include the recent LDAP changes in v1.0.0? They fix some LDAP fooness that shouldn't get out the door if at all possible.
I would be +1 to simply remove auth_ldap from APR 1.0, and reintroduce
the entire feature in the new APR 1.1 (which we can start development
on immediately.) And that presumes httpd 2.1/2.2 will depend on the
1.1 release of apr-util.
I hate to hold up 1.0 any further. I would hate to release apr-util as-in
even more. Thoughts or comments about my solution above?
Bill
Re: 1.0.0 RC5
Posted by Graham Leggett <mi...@sharp.fm>.
David Reid wrote:
> So I see. I'll tag & roll APR RC5 later on tonight and hopefully as soon
> as apr-util is patched for apr-config I'll be able to roll.
Would it be possible to include the recent LDAP changes in v1.0.0? They
fix some LDAP fooness that shouldn't get out the door if at all possible.
apr_ldap_init.c
apr_ldap.hw
apr_ldap.hnw
apr_ldap.h.in
NWGNUmakefile
Regards,
Graham
--
Re: 1.0.0 RC5
Posted by David Reid <da...@jetnet.co.uk>.
Justin Erenkrantz wrote:
> --On Friday, July 30, 2004 8:33 AM -0400 Ryan Bloom <rb...@gmail.com>
> wrote:
>
>> However, we need to remove the APR_STATUS_IS_SUCCESS macro before 1.0
>> goes
>> out, because otherwise we are stuck with it for a very long time.
>
>
> It's gone now. ;-)
So I see. I'll tag & roll APR RC5 later on tonight and hopefully as soon
as apr-util is patched for apr-config I'll be able to roll.
> I've got to fix up httpd 2.1 now and then I'll review Max's patch. --
Thanks Justin.
david