Re: 1.0.0 RC5
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 -- smime.p7s Description: S/MIME Cryptographic Signature
Re: 1.0.0 RC5
The prototype for apr_ldap_info() in apr_ldap_init.c is missing. It needs to be added to apr_ldap_init.h Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com Graham Leggett [EMAIL PROTECTED] Tuesday, August 03, 2004 7:03:36 PM 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
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
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
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
--On Tuesday, August 3, 2004 12:16 AM -0400 Cliff Woolley [EMAIL PROTECTED] 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
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
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
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
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. +1, V3 has been around for a while. Let's support V2 later if needed. My bet is that it won't be needed. Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com Graham Leggett [EMAIL PROTECTED] Tuesday, August 03, 2004 1:07:47 PM 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
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
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 -- smime.p7s Description: S/MIME Cryptographic Signature
Re: 1.0.0 RC5
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
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 -- smime.p7s Description: S/MIME Cryptographic Signature
Re: 1.0.0 RC5
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 -- smime.p7s Description: S/MIME Cryptographic Signature
Re: 1.0.0 RC5
At 08:18 AM 7/30/2004, David Reid 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. There were win32 comments from Brane? Is someone going to commit the changes needed? What changes? Note in apr_errno : #define APR_FROM_OS_ERROR(e) (e == 0 ? APR_SUCCESS : e + APR_OS_START_SYSERR) #define APR_TO_OS_ERROR(e) (e == 0 ? APR_SUCCESS : e - APR_OS_START_SYSERR) #define apr_get_os_error() (APR_FROM_OS_ERROR(GetLastError())) #define apr_set_os_error(e) (SetLastError(APR_TO_OS_ERROR(e))) #define apr_get_netos_error() (APR_FROM_OS_ERROR(WSAGetLastError())) #define apr_set_netos_error(e) (WSASetLastError(APR_TO_OS_ERROR(e))) so as you see we already fold 0 : 0 but ... oh my what''s this in the OS2 section??? // XXX deprecated #define APR_STATUS_IS_ETIMEDOUT(s) ((s) == APR_ETIMEDOUT \ || (s) == APR_OS_START_SYSERR + SOCETIMEDOUT) Bad comment form and a left-over deprecation? I'm out of week, but someone please squish that :-/ Bill
Re: 1.0.0 RC5
Justin Erenkrantz wrote: --On Friday, July 30, 2004 8:33 AM -0400 Ryan Bloom [EMAIL PROTECTED] 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
Re: 1.0.0 RC5
On Jul 30, 2004, at 7:33 AM, Ryan Bloom wrote: I don't understand why this is still being discussed. The patch makes sense, it solves a real problem and just needs to be committed and tested. +1 on adding it to 1.0.0RC5 so that we can get the release out. 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. +1 on both counts. -Fitz