537089 - make it possible to use system provided mozldap

Status: RESOLVED WONTFIX

(MailNews Core :: Build Config, defect)

Description

[Wolfgang Rosenauer [:wolfiR]]

Attachment: possible patch

Some Linux distributions ship mozldap libraries in separate packages already (e.g. for 389ds or probably others).
It probably makes sense to provide a way to enable the usage of the system provided mozldap libraries in SeaMonkey and Thunderbird builds.

An initial patch which apparently works is attached.

Comment 1

[Mark Banner (:standard8)]

Sorry for not getting back to this earlier. As I think I mentioned to you on irc, my biggest issue is that we're not actually using version 6.0.6 at the moment, but an upgraded version.

Whilst there's not much change between 6.0.6 and current, I'd be a bit dubious of "downgrading" as there's a couple of fixes in there I think we should keep.

So the obvious route is to get the LDAP c-sdk guys to push out 6.0.7.

Here comes the next problem - when I was producing the latest version that TB uses, I discovered that HEAD didn't compile on Windows, and so I worked around it. I've just set a try server build off to try and determine whether that's still true or not. Once that's complete, I'll make sure appropraite bugs are filed and marked as blocking this one.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 419415 [details] [diff] [review]
possible patch

> dnl ========================================================
>+dnl = If mozldap was not detected in the system, 
>+dnl = use the one in the source tree (mozilla/directory/c-sdk)
>+dnl ========================================================

The comment makes it sound like we'll build with the system version by default, but the code looks like it only builds with the system version if --enable-system-mozldap is specified (I agree with the code version).

Comment 3

[Wolfgang Rosenauer [:wolfiR]]

Thanks for the update. The current discussion within (parts of) openSUSE is more against the idea anyway because they rather try to standardize on openldap and proposed to replace mozldap with openldap within mozilla. It probably still makes sense to have the switch if there are no blocking issues on that but my usecase disappeared for the moment.

And yes, it was meant to only use system lib if explicitely asked for. I probably was unclear in the comment

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 419415 [details] [diff] [review]
possible patch

>+  if test -n "$SYSTEM_MOZLDAP"; then
>+      PKG_CHECK_MODULES(MOZLDAP, mozldap >= 6.0.6,
>+        SYSTEM_MOZLDAP=1, SYSTEM_MOZLDAP=)
>+  fi

Can you also separate out the version so that its at the top of the file in the section titled (to make it easier to find to update it):

dnl Set the minimum version of toolkit libs used by mozilla

Comment 5

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch #2

adressing comments

Comment 6

[Mark Banner (:standard8)]

Attachment: Updated patch

Sorry for the long delay in this, now that LDAP is sorted out, I think we can proceed with this.

I've updated the patch to fix bitrot and address some review comments, namely:

- Update requirement to the new version of LDAP
- Use --with-system-ldap rather than --enable-system-ldap (as this fits better with our other system flags)
- Make the changes to the LDAP_CFLAGS and LDAP_LIBS apply for any platform if building with system ldap (not just linux).

Wolfgang can you take a look at this and see if it still works? It works fine for the non-system case...

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 502007 [details] [diff] [review]
Updated patch

I think unless someone specifically wants this (and afaik no-one does), then I'm going to wontfix this for now. If we decide its wanted later, we can always re-open.

Comment 8

[Mark Banner (:standard8)]

(we might also reconsider it if we were to switch to openldap which would more likely be on a system already).