98924 - LDAP should use module specific include dir

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect)

Description

[cls]

LDAP should follow NSPR's example and use a module specific directory for its
public headers; private headers, of course, should not be exported.  This will
put LDAP in sync with the other efforts to modularize headers.  Since LDAP uses
a separate build system, I don't think REQUIRES should be used to find those
headers.

Comment 1

[cls]

Attachment: Install ldap headers in dist/include/ldap

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

r=dmose

Comment 3

[cls]

Patch has been checked in.  dmose says that a similar change needs to be made to
new ldap branch so reassigning to leif.

Comment 4

[cls]

Changing product.

Comment 5

[cls]

need this for win32 too

Comment 6

[cls]

Attachment: Install win32 ldap headers in DIST_XP\include\ldap

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

r=dmose in that cls' win32 changes look reasonable; I don't know the windows
build system well enough to be able to review them for completeness.

Comment 8

[Alec Flett]

why dont we just use REQUIRES in the directories that use ldap instead of adding
it to config.mak? I kind of thought nspr was done that way because we don't
really need to track nspr dependencies because it's so fundamental

Comment 9

[cls]

That is probably why NSPR is done that way.  Someday when we have more time to
worry about correctness, I'd like to change that.  IMO, REQUIRES only applies to
the modules that are built as part of the Mozilla core build system.  This is
why ZLIB/JPEG/PNG/MNG_REQUIRES are only set when building the mozilla version of
the packages.  Otherwise, we rely upon setting <mod>_CFLAGS to pull the proper
headers.  LDAP should be treated the same way since it is a completely separate
entity (as opposed to zlib which has been adapted to our build system).

Adding the ldap include path to config.mak was just for convenience.  We can not
add it there and just add it to the handful of makefiles that will use it
(directory/xpcom/* & mailnews/addrbook/* iirc).

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Only adding it to the dirs where it's needed would avoid adding unnecessary IO
cycles to the build.  The directory/xpcom/base subtree should be the ONLY place
that needs to include those files directly.

Comment 11

[cls]

Include directly, yes.  But since nsLDAP.h & friends pull in ldap.h, the modules
that require mozldap will also require ldap.  It's still only a handful of
makefiles though.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

nsLDAP.h is going to go away; see bug 92650.

Comment 13

[cls]

Sorry, that should have been nsLDAPURL.h & friends (the ones exported from
directory/xpcom/base/src/) not nsLDAP.h

Comment 14

[cls]

Attachment: Add LDAP_CFLAGS to win32 & use as necessary

Comment 15

[cls]

Attachment: Only use LDAP_CFLAGS as necessary on unix

Comment 16

[Alec Flett]

I like that better. I still like the idea of using REQUIRES somehow, because I'm
developing tools that make use of the value of REQUIRES.

Comment 17

[Dan Mosedale (:dmosedale, :dmose)]

cls: nothing from the src directory should really have to be exported, I don't
think.  It's most likely a bug I introduced by copying other win32 makefiles,
although conceivably there's some weird win32 build system lossage that I was
hacking around.

Comment 18

[cls]

You're right. I went digging some more and it wasn't that dir, lxr shows that
addrbook directly includes ldap.h

/mailnews/addrbook/src/nsAbLDAPProperties.h, line 35 -- #include "ldap.h"

Comment 19

[Dan Mosedale (:dmosedale, :dmose)]

cls: that's definitely a bug; that code should not be including ldap.h.

Comment 20

[cls]

Attachment: Remove unneeded ldap.h include from addrbook

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

r=dmose on all three, minus the hunks pertaining to
mailnews/addrbook/src/{makefile.win,Makefile.in}

Comment 22

[Alec Flett]

sr=alecf on all three.

Comment 23

[cls]

Those 3 patches have been checked in.

Comment 24

[Leif Hedstrom]

Reassign to dmose.

Comment 25

[scottputterman]

Moving out of 0.9.7.  Feel free to move them past 0.9.8.

Comment 26

[Dan Mosedale (:dmosedale, :dmose)]

Mass change: pushing out 0.9.8 bugs, as I'm concentrating feature work now.

Comment 27

[scottputterman]

Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to [e-mail].  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.

Comment 28

[Dan Mosedale (:dmosedale, :dmose)]

This bug was fixed with the landing of the LDAP C SDK v5.