On Mon, Jun 27, 2011 at 03:19:37PM +0200, Graham Leggett wrote: > mod_ldap - An LDAP shared memory cache > mod_authnz_ldap - A user of the LDAP shared memory cache > > The LDAP API exposes way more functionality than mod_ldap exposes, > so while you may have fixed the problem for the special case that is > mod_authnz_ldap, you won't have fixed the problem for any other > module that makes LDAP calls directly.
I don't see how this can be the basis of a veto. You are stating that there exists a problem which this change does not fix; but that problem existed in the status quo ante, and it *was not the problem this change was intended to fix*. > The v1.x LDAP API was removed from APR because it was an incomplete > abstraction, and for this exact same set of reasons it is > inappropriate to add it to httpd. The API provided by mod_ldap was already an "incomplete abstraction" - this is not something that has been changed in this commit to httpd, so I cannot see how this can be the basis of a veto, as above. The consensus vote in APR concluded precisely that the code was more appropriate for httpd than APR; you cannot twist the logic of that to pretend it implies the opposite. > Over and above that, given that httpd has no library component > common to all modules (well, it has, it's called APR), and given > that the httpd core is inappropriate for an LDAP portability > library, the only place to put the LDAP bindings is in mod_ldap, > which means we either have to accept the module ordering problems > that result, or we have to create optional functions for every > single LDAP API call, and if we do that, we end up solving the > reason apr_ldap was removed from APR in the first place. a) "httpd core is inappropriate for an LDAP portability library" -> this is an opinion, not technical justification for a veto. b) "we end up solving the reason" I don't understand that sentence, can you rephrase? > In addition, we moved all portability code out of httpd in v2.0, it > is a huge step backwards to go back on that effort. It is really > ugly to have apr_xml, apr_dbd, apr_crypto and apr_dbm in APR, and > then ap(r)_ldap somewhere else. "it's ugly"... sure. I agree with that, but I also think this change is a significant improvement. Merely stating "it is ugly" is not sufficient basis for a veto. > This is the essence of my veto. I don't see technical basis for a veto here, unless you can clarify. Regards, Joe