Upgrade LDAP C-SDK to 6.0.x on trunk for Address Book.

RESOLVED FIXED in Thunderbird 3.0a3

Status

defect
P2
normal
RESOLVED FIXED
13 years ago
11 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3.0a3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(10 attachments, 6 obsolete attachments)

3.32 KB, patch
Details | Diff | Splinter Review
2.64 KB, patch
mcs
: review+
Details | Diff | Splinter Review
119.20 KB, patch
Details | Diff | Splinter Review
4.81 KB, patch
Details | Diff | Splinter Review
4.81 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
856 bytes, patch
benjamin
: review+
Details | Diff | Splinter Review
7.61 KB, patch
Details | Diff | Splinter Review
42.85 KB, patch
mcs
: review+
Details | Diff | Splinter Review
1.19 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
282 bytes, patch
dmose
: review+
Details | Diff | Splinter Review
Now that the Mozilla LDAP C-SDK 6.0 has been released we should be upgrading to it to pick up the latest fixes, especially the IPv6 ones (bug 228439).

Do we want to stay with the branching from the release tag, or just update to the release tag? I'm not sure if there's anything historical as to why.

I've got the patches, though I'd like a couple of volunteers to test them out on Mac and Windows (they work for me on Linux).

I'd also like someone to help with checking it in. I can't guarantee having a few hours available to help build engineers if tinderboxes start breaking, which I think they may do again with this upgrade (like when we upgraded to 5.1.7).
This patch will update a Mozilla tree to use LDAP c-sdk version 6.0 from the tag not a branch. Its good enough for just checking ldap still works with version 6.0 on the various trees (and to pick up any potential problems).
Adding Mark Smith to the CC list.

Mark, what are some of the reasons the client has used a branch of the ldapcsdk instead of just using the trunk?
The reasons for Mozilla applications such as Seamonkey or TBird to use a branch or a static tag is stability:  we wanted to keep LDAP C SDK development a little bit separate from Seamonkey/Thunderbird (much like is done for NSPR and NSS).

I think an ideal solution would be for the applications to use a static tag (to the trunk), updating it when appropriate, and only branching when necessary.  But historically the updates have happened infrequently, which lead to a need for a branch in order to apply minor changes such as porting fixes.
Switching to a static tag on the trunk sounds like a fine plan to me.
(In reply to comment #4)
> Switching to a static tag on the trunk sounds like a fine plan to me.

That's good, I'll post the fact to m.d.planning and a couple of other places later so that people know what's happening.

I guess we'll now be picking up 6.0.2 at least now as well.
Summary: Upgrade LDAP C-SDK to 6.0 on trunk for Address Book. → Upgrade LDAP C-SDK to 6.0.x on trunk for Address Book.
The static trunk tag for mozldap version 6.0.2 is LDAPCSDK_6_0_2_RTM.  This version also contains the result of the merging of the forked Sun code back into Mozilla.
http://wiki.mozilla.org/LDAP_C_SDK
and
http://wiki.mozilla.org/LDAPSunMerge
Updates to 6.0.2.

David/Karsten, would you two be able to try this out on Windows/Mac? - I'd like to know if it at least compiles. You'll (of course) need to do a pull after applying the patch.

I expect there may be a conflict when checking out mozilla/directory/c-sdk/ldap/libraries/libutil/Makefile if you're building in-tree. Its due to that makefile being modified by the old make scripts and is removed in the latest version of the c-sdk.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Mark, I don't run the trunk very often at the moment, since we're concentrating on 2.0
Attachment #244831 - Attachment is obsolete: true
Posted patch OS/2 diff (obsolete) — Splinter Review
There is no os2sock.h file... I guess that someone added it because of the winsock.h above in the windows section.  
This builds but I am not sure how to actually test it.  The only thing I knew for sure was using ldap that I had required ldap experimental to be turned on and I think that is currently still broken.
I sent Mark an e-mail. I tried to build with his patch on my intel Mac with Mac OS X 10.4.8 and it failed. I think I did this right. I have been building for a little while. On the other hand, if I look at the particular error, I would not be shocked if I did not do something right.

The stuff at the bottom of the log is:

c++  -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -fpascal-strings -no-cpp-precomp -fno-common -fshort-wchar -I/Developer/Headers/FlatCarbon -pipe  -DNDEBUG -DTRIMMED -O -fPIC  -o libaccessibility.dylib  nsAccessibilityFactory.o  nsAccessNode.o nsAccessibleEventData.o nsDocAccessible.o nsOuterDocAccessible.o nsAccessibilityAtoms.o nsAccessibilityService.o nsAccessible.o nsAccessibleTreeWalker.o nsBaseWidgetAccessible.o nsFormControlAccessible.o nsRootAccessible.o nsCaretAccessible.o nsTextAccessible.o nsHTMLAreaAccessible.o nsHTMLFormControlAccessible.o nsHTMLImageAccessible.o nsHTMLLinkAccessible.o nsHTMLSelectAccessible.o nsHTMLTableAccessible.o nsHTMLTextAccessible.o nsHyperTextAccessible.o nsAccessNodeWrap.o nsDocAccessibleWrap.o nsRootAccessibleWrap.o nsAccessibleWrap.o mozAccessible.o mozDocAccessible.o mozActionElements.o mozTextAccessible.o nsXULAlertAccessible.o nsXULColorPickerAccessible.o nsXULFormControlAccessible.o nsXULMenuAccessible.o nsXULSelectAccessible.o nsXULTabAccessible.o nsXULTextAccessible.o nsXULTreeAccessible.o nsXFormsAccessible.o nsXFormsFormControlsAccessible.o nsXFormsWidgetsAccessible.o    -Wl,-dead_strip -L../../dist/bin -L../../dist/lib -lgkgfx ../../dist/lib/libunicharutil_s.a -L../../dist/bin -lxpcom -lxpcom_core -L../../dist/bin -L../../dist/lib -lplds4 -lplc4 -lnspr4  -framework Cocoa -Wl,-exported_symbols_list -Wl,../../build/unix/gnu-ld-scripts/components-export-list -bundle    
chmod +x libaccessibility.dylib
/Users/ray/mo/mail/mozilla/config/nsinstall -L /Users/ray/mo/mail/mozilla/accessible/build -m 755 libaccessibility.dylib ../../dist/bin/components
: ../../dist/bin/components/libaccessibility.dylib
tier_toolkit: directory/c-sdk chrome profile xpfe/bootstrap/appleevents xpfe toolkit/components  extensions/spellcheck toolkit xpinstall security/manager extensions/pref toolkit/library directory/xpcom testing/mochitest
make[2]: *** No targets specified and no makefile found.  Stop.
make[1]: *** [tier_toolkit] Error 2
make: *** [default] Error 2
Mark,

mail builds just fine on Mac OS 10.4.8 with your patch. Address Book seems
to be working alright with 6.0.2, tried both PLAIN and SSL. built it using
10.3.9 SDK so it should work on Panther too. this is on PPC btw. on a side
note it looks like you guys are not using libssldap but rather doing some 
private stuff via xpcom instead. this is probably to do with PSM and certs
handling etc but it might be worth looking into using libssldap as we have
some new kewl stuff there like Start TLS extended operation which is kinda
popular these days or so it seems and it would be great to have supported.
Was able to test and found that there was another change that was missed, with these changes it did work.
Attachment #253900 - Attachment is obsolete: true
Comment on attachment 254047 [details] [diff] [review]
New OS/2 diff (checked in to c-sdk head)

Mark, can you review & approve this for checkin to the c-sdk trunk please?

Mark/Rich - when we get this patch in, could we do a 6.0.3 release to incorporate it? Although OS2 isn't a tier 1 platform, I'd like to continue to support it when we do switch for SM/TB.
Attachment #254047 - Flags: review?(mcs)
Comment on attachment 254047 [details] [diff] [review]
New OS/2 diff (checked in to c-sdk head)

This looks okay to me.  I'll let Rich comment on the 6.0.3 release (I am not sure what is involved beyond someone applying a tag).
Attachment #254047 - Flags: review?(mcs) → review+
We could certainly go ahead and tag the tree with LDAPCSDK_6_0_3_RTM.  However, we're still trying to figure out what exactly constitutes a release.  src releases are easy - binary releases are not.
Attachment #254047 - Attachment description: New OS/2 diff → New OS/2 diff (checked in to c-sdk head)
(In reply to comment #15)
> We could certainly go ahead and tag the tree with LDAPCSDK_6_0_3_RTM.  However,
> we're still trying to figure out what exactly constitutes a release.  src
> releases are easy - binary releases are not.
> 
Well it'll be about a week before I'll be able to do anything more on this (= set up patches and get diffs of configs etc for benjamin's approval). Not sure what your timescales are, but I'd certainly like to look at doing this once I get time.
(In reply to comment #16)
> (In reply to comment #15)
> > We could certainly go ahead and tag the tree with LDAPCSDK_6_0_3_RTM.  However,
> > we're still trying to figure out what exactly constitutes a release.  src
> > releases are easy - binary releases are not.
> > 
> Well it'll be about a week before I'll be able to do anything more on this

Rich, could we set up a 6.0.3 when you're ready, even if its just a src release still? I'll try and find the time to do the necessary work to get the update onto the trunk for TB/SM to use, and I think its probably a good time to upgrade.
Sure.  We have some more post 6.0.2 fixes to get in as well.
The following bonsai query lists all of the changes since 6.0.2:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=DirectorySDKSourceC&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-17+00%3A00%3A00&maxdate=2007-03-08+00%3A00%3A00&cvsroot=%2Fcvsroot

I just need to update the version in build.mk to 6.0.3 and we're ready to go.
(In reply to comment #18)
> Sure.  We have some more post 6.0.2 fixes to get in as well.
> The following bonsai query lists all of the changes since 6.0.2:
> http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=DirectorySDKSourceC&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-17+00%3A00%3A00&maxdate=2007-03-08+00%3A00%3A00&cvsroot=%2Fcvsroot
> I just need to update the version in build.mk to 6.0.3 and we're ready to go.

Excellent, the list of changes look reasonable and its certainly worth picking them up.
The Mozilla LDAP C SDK v6.0.3 has been tagged.  Details here - http://wiki.mozilla.org/LDAP_C_SDK#Latest_News_-_3.2F13.2F2007
Last time we upgraded the ldap version that we used, I was asked to supply the diffs of the build files between the two versions, so I'm pre-empting that here.
Posted patch Update to c-sdk version 6.0.3 (obsolete) — Splinter Review
Benjamin, can you review this from a build config perspective? If you want to see the diffs relevant to building between the two versions, I've already included it in attachment 259137 [details] [diff] [review].

Scott: This is to upgrade trunk to the latest LDAP c-sdk. It'll bring us up to date especially with picking up the full version of the ipv6 fix.


Not sure when I'll be able to land this yet as it'll need the tinderboxes watching. Hopefully in the week before
Attachment #259138 - Flags: superreview?(mscott)
Attachment #259138 - Flags: review?
Comment on attachment 259138 [details] [diff] [review]
Update to c-sdk version 6.0.3

Bugzilla is annoying sometimes. Benjamin, please see my previous comment.
Attachment #259138 - Flags: review? → review?(benjamin)
Comment on attachment 259138 [details] [diff] [review]
Update to c-sdk version 6.0.3

Looks good to me pending any comments by Benjamin.
Attachment #259138 - Flags: superreview?(mscott) → superreview+
6.0 requires a new runtime library - libldif60.  This should not add much to the package size, since this was previously linked statically into libldap.  Any ldif parsing code in thunderbird should be able to use this code (ldif.h) instead of any custom code written to handle ldif parsing (if any), so there is an opportunity for code removal.
Comment on attachment 259138 [details] [diff] [review]
Update to c-sdk version 6.0.3

(In reply to comment #25)
> 6.0 requires a new runtime library - libldif60.

Well I'll link against it now, I'll also update the packaging lists which I'd completely forgotten about.
Attachment #259138 - Flags: review?(benjamin)
This looks like a nicely bundled pack of functionality. Where are the tests that go along with it?
(In reply to comment #27)
> This looks like a nicely bundled pack of functionality. Where are the tests
> that go along with it?
> 
AFAIK there are no automatic/manual tests for the LDAP C-SDK.
(In reply to comment #25)
> 6.0 requires a new runtime library - libldif60.

Rich, does liblber50.so (or even 60) ever get generated? Its just that SeaMonkey's unix packaging file references it but not in either of the windows packaging files that SeaMonkey or Thunderbird use. Thanks.

liblber60 is built as a static library only - clients apps usually don't need direct access to liblber.
Add the required packaging changes.

Benjamin, can you review this from a build config perspective? If you want to
see the diffs relevant to building between the two versions, I've already
included it in attachment 259137 [details] [diff] [review].
Attachment #259138 - Attachment is obsolete: true
If new stuff is getting landed here, can these tests get some love also?

http://www.mozilla.org/directory/csdk-tests.html
(In reply to comment #33)
> If new stuff is getting landed here, can these tests get some love also?
> 
> http://www.mozilla.org/directory/csdk-tests.html
> 

This is landing existing code on trunk. As we've never had formally up-to-date tests before, lets take any discussion on tests to the mozilla.dev.tech.ldap.
Corrects the linking options specified in configure.in. 

I'm still testing this on linux, if anyone can test on windows/mac that'd be useful. You'll need to make sure you do a checkout or pull_all after applying the patch and also re-running autoconf to get the configure.in changes into configure. It may also be worth trashing $(objdir)/dist/libs, $(objdir)/dist/bin and $(objdir)/directory just to make sure you haven't got any old 5.x libs lying around.
Attachment #259435 - Attachment is obsolete: true
my debug windows build failed with this error:

======= making ./nsldap32v60.dll
link -DEBUG -nologo -MAP -DLL -PDB:NONE  -SUBSYSTEM:CONSOLE   wsock32.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib rpcrt4.lib uuid.lib winmm.lib C:/mozilla-build/msys/tbirdtrunk/mozilla/dist/lib/nsldif32v60.lib C:/mozilla-build/msys/tbirdtrunk/mozilla/dist/lib/nslber32v60.lib -out:"nsldap32v60.dll" ./abandon.obj ./add.obj ./authzidctrl.obj ./bind.obj ./cache.obj ./charray.obj ./charset.obj ./compare.obj ./compat.obj ./control.obj ./countvalues.obj ./delete.obj ./disptmpl.obj ./dsparse.obj ./error.obj ./extendop.obj ./free.obj ./freevalues.obj ./friendly.obj ./getattr.obj ./getdn.obj ./getdxbyname.obj ./geteffectiverightsctrl.obj ./getentry.obj ./getfilter.obj ./getoption.obj ./getvalues.obj ./memcache.obj ./message.obj ./modify.obj ./open.obj ./os-ip.obj ./proxyauthctrl.obj ./psearch.obj ./pwmodext.obj ./pwpctrl.obj ./referral.obj ./regex.obj ./rename.obj ./request.obj ./reslist.obj ./result.obj ./saslbind.obj ./sbind.obj ./search.obj ./setoption.obj ./sort.obj ./sortctrl.obj ./srchpref.obj ./tmplout.obj ./ufn.obj ./unbind.obj ./unescape.obj ./url.obj ./userstatusctrl.obj ./utf8.obj ./vlistctrl.obj ./whoami.obj ./dllmain.obj ./mozock.obj  -DEF:C:/mozilla-build/msys/tbirdtrunk/mozilla/directory/c-sdk/ldap/libraries/libldap/../msdos/winsock/nsldap32.def wsock32.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib rpcrt4.lib uuid.lib winmm.lib C:/mozilla-build/msys/tbirdtrunk/mozilla/dist/lib/nsldif32v60.lib C:/mozilla-build/msys/tbirdtrunk/mozilla/dist/lib/nslber32v60.lib
C:/mozilla-build/msys/tbirdtrunk/mozilla/directory/c-sdk/ldap/libraries/libldap/../msdos/winsock/nsldap32.def(39) : warning LNK4017: DESCRIPTION statement not supported for the target platform; ignored
   Creating library nsldap32v60.lib and object nsldap32v60.exp
nslber32v60.lib(io.obj) : error LNK2019: unresolved external symbol _lber_bprint referenced in function _ber_flush@12
nsldap32v60.dll : fatal error LNK1120: 1 unresolved externals
make[6]: *** [nsldap32v60.dll] Error 96

I haven't had a chance to look further into it. I'll try a clean build in mozilla/directory first.
(In reply to comment #36)
> my debug windows build failed with this error:
>    Creating library nsldap32v60.lib and object nsldap32v60.exp
> nslber32v60.lib(io.obj) : error LNK2019: unresolved external symbol
> _lber_bprint referenced in function _ber_flush@12

I'm not really sure how window's .def files work, but could this be because lber_bprint isn't defined in http://lxr.mozilla.org/mozilla/source/directory/c-sdk/ldap/libraries/msdos/winsock/nsldap32.def ? lber_bprint is a debug function only, so I can understand why its not in there.
Since the lber_bprint() function is internal to nsldap32v60.dll I do not think it needs to be added to the .def file.  Definitely try a clean build.
sorry, yes, a clean build in mozilla/directory fixed the problem.
v4 Fixes linking issue on linux.

Benjamin, can you review this from a build config perspective? If you want to see the diffs relevant to building between the two versions of the c-sdk, I've already included it in attachment 259137 [details] [diff] [review] [details].
Attachment #259913 - Flags: review?(benjamin)
Attachment #259913 - Flags: review?(benjamin) → review+
For general info: I'll be checking this in either at the weekend or sometime next week as I'm expecting some bustage (due to one of the old Makefiles being modified on tinderboxes but cvs removed in the new branch) on the tinderboxes so I want to make sure I've got plenty of time to watch/fix it all.
Benjamin, I fixed bustage from the checkin of attachment 259913 [details] [diff] [review] with this patch - basically it copied the NSPR way of pulling so that if using a release tag then we don't pull by date.

I'm requesting your review to check you're happy with the additional change.
Attachment #260129 - Flags: review?(benjamin)
Comment on attachment 260129 [details] [diff] [review]
client.mk follow up

Yeah, I'm surprised we didn't do this before. Does the ldap code use _branch or _BRANCH?
Attachment #260129 - Flags: review?(benjamin) → review+
(In reply to comment #43)
> (From update of attachment 260129 [details] [diff] [review])
> Yeah, I'm surprised we didn't do this before. Does the ldap code use _branch or
> _BRANCH?
> 
Well we have done _branch before, at the moment we're using _RTM for the release, and HEAD. I see no reason why it can't be required to be _BRANCH now.


On a separate note I've still got bustage on windows tinderboxes. I think for some reason it was decided that cygwin-wrapper wasn't needed for some reason. However the bustage seems to indicate it is required. As there hasn't been much changed since the last c-sdk release (only one checkin - Bug 372858) I'm going to temporarily change client.mk to pull HEAD and fix the configure.in to see if that works. We'll have to figure out if this is the correct fix later.
I tried landing version 6.0.3 on trunk the other day. I had problems with two bustages:

The first was on windows I think it was because the cygwin-wrapper variable had been cleared in directory/c-sdk/configure.in (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/directory/c-sdk/configure.in&rev=5.54&mark=939-941#939) with no particular explanation apart from its not really needed anymore. I tested going to HEAD and temporarily putting it back in and that didn't work (its now backed out again), but I think that was because configure wasn't re-run for some reason, and then I ran out of time.

The second problem was one of the mac tinderboxes failed in configure:

checking whether the C++ compiler (g++-4.0 -arch i386  ) works... no
configure: error: installation or configuration problem: C++ compiler cannot create executables.
configure: error: /Users/robert/tinderbox/SeaMonkey-exp/Darwin_8.8.0_Depend/mozilla/directory/c-sdk/configure failed for directory/c-sdk

Full log: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1175277540.1175278631.2972.gz&fulltext=1

Does anyone have any idea why that failed? It passed on the main SeaMonkey & Thunderbird boxes, just not that one. 

The possibly significant difference is the ones that passed use SDK MacOSX10.3.9, whereas the one that failed uses MacOSX10.4u.


My current plan is to see if we can fix mac, and then create a 6.0.3 branch and attempt to get that on SeaMonkey & Thunderbird trunk and see if we can a) fix the cygwin-wrapper and b) fix the mac problem.

I'd like to try again on Friday as that's the most convenient time I know I'll have in the next week or so, so any thoughts would be useful.
(In reply to comment #45)
> The first was on windows I think it was because the cygwin-wrapper variable had
> been cleared in directory/c-sdk/configure.in
> (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/directory/c-sdk/configure.in&rev=5.54&mark=939-941#939)
> with no particular explanation apart from its not really needed anymore. I
> tested going to HEAD and temporarily putting it back in and that didn't work
> (its now backed out again), but I think that was because configure wasn't
> re-run for some reason, and then I ran out of time.

cygwin-wrapper is definitely not needed for standalone ldap c sdk builds - is it still required for building the ldap c sdk as part of the client?  If so, we can add it back.
(In reply to comment #46)
> cygwin-wrapper is definitely not needed for standalone ldap c sdk builds - is
> it still required for building the ldap c sdk as part of the client?  If so, we
> can add it back.

I think it may be - it certainly seems to be used as part of the client like that. I'd like to try it on a branch with the client before we put it back in though (and hopefully try a mac fix at the same time, if there is one).
(In reply to comment #45)

i found it strange that Thunderbird compiled fine with GCC4 and 10.4SDK,
last time i tried and it was couple of month back you had to have GCC3
and 10.3SDK and use them for building otherwise the build doesnt go too
far. you can switch default GCC with gcc_select and 10.3SDK ships with
XCode package. 

> The second problem was one of the mac tinderboxes failed in configure:
> 
> checking whether the C++ compiler (g++-4.0 -arch i386  ) works... no
> configure: error: installation or configuration problem: C++ compiler cannot
> create executables.
> configure: error:
> /Users/robert/tinderbox/SeaMonkey-exp/Darwin_8.8.0_Depend/mozilla/directory/c-sdk/configure
> failed for directory/c-sdk
> 
> Full log:
> http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1175277540.1175278631.2972.gz&fulltext=1
> 
> Does anyone have any idea why that failed? It passed on the main SeaMonkey &
> Thunderbird boxes, just not that one. 
> 
> The possibly significant difference is the ones that passed use SDK
> MacOSX10.3.9, whereas the one that failed uses MacOSX10.4u.
(In reply to comment #48)
> (In reply to comment #45)
> i found it strange that Thunderbird compiled fine with GCC4 and 10.4SDK,
> last time i tried and it was couple of month back you had to have GCC3
> and 10.3SDK and use them for building otherwise the build doesnt go too
> far. you can switch default GCC with gcc_select and 10.3SDK ships with
> XCode package. 

Ok, I've done some more investigation. It turns out that the client builds (Thunderbird & SeaMonkey) are now doing universal binaries for mac. This effectively means one build on ppc (which succeeded) and one cross-compile on i386. In the configure output this yeilds:

checking host system type... powerpc-apple-darwin8.8.0
checking target system type... i386-apple-darwin8.8.0
checking build system type... powerpc-apple-darwin8.8.0

The configure logic says we're not doing a cross-compile:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/directory/c-sdk/configure.in&rev=5.56&mark=580-590#569

The main mac tinderboxes for Thunderbird & SeaMonkey seem to have compilers that cope with this, the one that was playing up didn't.

Note that NSPR & the main mozilla configure scripts do pick this case up as a cross-compilation. (However...)

Rich, I think you changed this in bug 355434, I'm unsure from your checkin comments if you changed it fully as you meant to. Can you have another look and let me know what you think?
(In reply to comment #49)
> Ok, I've done some more investigation. It turns out that the client builds
> (Thunderbird & SeaMonkey) are now doing universal binaries for mac. This
> effectively means one build on ppc (which succeeded) and one cross-compile on
> i386. In the configure output this yeilds:
> 
> checking host system type... powerpc-apple-darwin8.8.0
> checking target system type... i386-apple-darwin8.8.0
> checking build system type... powerpc-apple-darwin8.8.0
> 
> The configure logic says we're not doing a cross-compile:
> 
> http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/directory/c-sdk/configure.in&rev=5.56&mark=580-590#569
> 
> The main mac tinderboxes for Thunderbird & SeaMonkey seem to have compilers
> that cope with this, the one that was playing up didn't.
> 
> Note that NSPR & the main mozilla configure scripts do pick this case up as a
> cross-compilation. (However...)
> 
> Rich, I think you changed this in bug 355434, I'm unsure from your checkin
> comments if you changed it fully as you meant to. Can you have another look and
> let me know what you think?

It looks like the logic that determines cross compilation will have to be different on darwin than on linux.

cross_compiling=no
dnl host must be specified to cross compile
if test -n "$host" ; then
    case $target in
    *-darwin*)
        if [ $host != $target ]; then
            cross_compiling=yes
        fi
    ;;
    *)
dnl host == build - do not cross compile
        if test "$host" = "$build" ; then
            cross_compiling=no
dnl host != build or host != target - do cross compile
        elif test "$host" != "$target" -o "$host" != "$build" ; then
            cross_compiling=yes
        fi
    ;;
    esac
fi
(In reply to comment #50)
> It looks like the logic that determines cross compilation will have to be
> different on darwin than on linux.

I'd be happy to give that a try. How do we want to proceed? I think its probably best if I put your suggested change and the cygwin-wrapper on a branch and temporarily try that. If that works, we can get the patches reviewed & put on trunk and then pick up a new release from that. I think I'd still prefer at this stage to end up with a stable release version of ldap c-sdk for the clients now that we know what the problems are.
(In reply to comment #51)
> I'd be happy to give that a try. How do we want to proceed? I think its
> probably best if I put your suggested change and the cygwin-wrapper on a branch
> and temporarily try that. If that works, we can get the patches reviewed & put
> on trunk and then pick up a new release from that. I think I'd still prefer at
> this stage to end up with a stable release version of ldap c-sdk for the
> clients now that we know what the problems are.

Yes, I agree.  Your proposal sounds fine.

Scott - we had a few problems with the original 6.0.3 of the c-sdk, so I've created a client branch and checked in the changes that we want to try on it. Here's the patch with the updated tag - hopefully I'll get to try this out on Friday.

If this works, the plan is to also pick up the next c-sdk release when it comes out.
Attachment #260686 - Flags: superreview?(mscott)
Comment on attachment 260686 [details] [diff] [review]
Update to c-sdk version 6.0.3 with client branch

I didn't test this patch, but the changes look good.

Quick question, do we need to add the v50 dlls to removed-files.in? 

http://mxr.mozilla.org/mozilla/source/mail/installer/removed-files.in

so the installer will remove the old dlls when installing over a previous version?
Attachment #260686 - Flags: superreview?(mscott) → superreview+
(In reply to comment #54)
> Quick question, do we need to add the v50 dlls to removed-files.in?

Yes.

> 
> http://mxr.mozilla.org/mozilla/source/mail/installer/removed-files.in
> 
> so the installer will remove the old dlls when installing over a previous
> version?
> 

Adds the missing removals for the old dll versions.
Attachment #260686 - Attachment is obsolete: true
I've just tried this again and the windows builds failed in the same manner as before. It is strange because although I put the cygwin-wrapper variable back in, it didn't show up the cygwin-wrapper in the logs, but I can't see why that would not happen correctly.
(In reply to comment #57)
> I've just tried this again and the windows builds failed in the same manner as
> before. It is strange because although I put the cygwin-wrapper variable back
> in, it didn't show up the cygwin-wrapper in the logs, but I can't see why that
> would not happen correctly.

Mark, can you post some relevant build log excerpts?
(In reply to comment #58)
> Mark, can you post some relevant build log excerpts?

This is the actual error section:

cd ldap; /usr/bin/make BUILD_DEBUG=optimize export
make[4]: Entering directory `/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/directory/c-sdk/ldap'
cd include; /usr/bin/make BUILD_DEBUG=optimize export
make[5]: Entering directory `/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/directory/c-sdk/ldap/include'
nsinstall -D c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/dist/public/ldap
perl /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/../build/replace.pl \
	LDAP_VENDOR_NAME="mozilla.org" \
		LDAP_VENDOR_VERSION="603" \
		< /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldap-standard-tmpl.h > c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/dist/public/ldap/ldap-standard.h
nsinstall -D c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/dist/public/ldap-private
nsinstall -R  -m 644 /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/disptmpl.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/lber.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldap.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldap-extension.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldap-platform.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldap-to-be-deprecated.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldap-deprecated.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldap_ssl.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldappr.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/ldif.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/iutil.h /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/directory/c-sdk/ldap/include/srchpref.h c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/build/dist/public/ldap
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\disptmpl.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\lber.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldap.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldap-extension.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldap-platform.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldap-to-be-deprecated.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldap-deprecated.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldap_ssl.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldappr.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\ldif.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\iutil.h: No such file or directory
nsinstall: \cygdrive\c\tinderbox\SeaMonkey-exp\WINNT_5.1_Depend\mozilla\directory\c-sdk\ldap\include\srchpref.h: No such file or directory
make[5]: *** [export] Error 3

This link is a dep build so a reasonable length log:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1175852340.1175854428.22773.gz&fulltext=1

This link is to a clobber build (kills the object dir) and quite long:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1175852220.1175856792.28402.gz&fulltext=1

Note at various times before the error section (before it gets into the c-sdk), there are lines such as:

/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/build/cygwin-wrapper ../../../dist/bin/xpt_link.exe _xpidlgen/appshell.xpt _xpidlgen/nsIAppShellService.xpt _xpidlgen/nsIWindowMediator.xpt _xpidlgen/nsIWindowMediatorListener.xpt _xpidlgen/nsIXULWindow.xpt _xpidlgen/nsIPopupWindowManager.xpt _xpidlgen/nsIXULBrowserWindow.xpt 
/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/build/cygwin-wrapper /cygdrive/c/moztools-static/bin/nsinstall -m 644 _xpidlgen/appshell.xpt ../../../dist/bin/components

which imply the cygwin-wrapper may be necessary/expected.
(In reply to comment #57)
> I've just tried this again and the windows builds failed in the same manner as
> before. It is strange because although I put the cygwin-wrapper variable back
> in, it didn't show up the cygwin-wrapper in the logs, but I can't see why that
> would not happen correctly.

I've just taken another look at directory/c-sdk/configure{,.in} it seems I missed the fact that there are two definitions of the cygwin-wrapper variable in the file, and I'd only put the first one back to what it was :-(

Hopefully I'll get chance at the weekend to give this another go with the client branch.
Ok, I've been able to retry the fix, and though I'm still checking one minor change to the cygwin-wrapper variable (included in this patch), I'm happy these are the changes we need to do on the c-sdk trunk.

Mark can you give your approval for these to go in on trunk?
Attachment #261406 - Flags: review?(mcs)
Comment on attachment 261406 [details] [diff] [review]
Build changes for c-sdk trunk (checked in)

OK.
Attachment #261406 - Flags: review?(mcs) → review+
Attachment #261406 - Attachment description: Build changes for c-sdk trunk → Build changes for c-sdk trunk (checked in)
Rich, I realise we haven't had long since the 6.0.3 release, but what's your opinion on doing a 6.0.4 one to pull in the changes I just checked into the trunk and using that as the new client release?

It'd be nice to get a proper working release matched up in the next month or so, but if you want to wait a while I'm happy to, I'll just formalise a couple of things in cvs and we'll use the 6.0.3 client branch in the interim.
(In reply to comment #63)
> Rich, I realise we haven't had long since the 6.0.3 release, but what's your
> opinion on doing a 6.0.4 one to pull in the changes I just checked into the
> trunk and using that as the new client release?

That's fine.  We can do a 6.0.4.

> It'd be nice to get a proper working release matched up in the next month or
> so, but if you want to wait a while I'm happy to, I'll just formalise a couple
> of things in cvs and we'll use the 6.0.3 client branch in the interim.

I think we should just go ahead and get these changes on the trunk for a 6.0.4.    I'm also currently working on doing a standalone sdk windows package, so I'll probably have some related changes to make that we will also want to get into a 6.0.4.
Depends on: 379184
No longer depends on: 379184
OS/2 needs this build break fix otherwise it can't link XPCOM. (VACPP is not used on OS/2 any more.)
Attachment #265301 - Flags: review?(benjamin)
Attachment #265301 - Flags: review?(benjamin) → review+
Attachment #265301 - Attachment description: fix SM build break on OS/2 → fix SM build break on OS/2 (checked into trunk)
Posted patch Update to c-sdk version 6.0.4 (obsolete) — Splinter Review
Updates the tag to the latest release version - 6.0.4 so we can pick up the latest changes (though I think Dan's already pulled across most of these onto the 6.0.3 client branch).
Attachment #271367 - Flags: superreview?(mscott)
Attachment #271367 - Flags: review?(mscott)
Attachment #271367 - Flags: superreview?(mscott)
Attachment #271367 - Flags: superreview+
Attachment #271367 - Flags: review?(mscott)
Attachment #271367 - Flags: review+
Patch checked in - so we're now using 6.0.4 with its static release tag. Therefore this bug is fixed.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
I'm reopening this - it appears that fixing bug 385091 has caused some bustage for some msys/cygwin compilation environment variants, so I backed it out from the client branch, until we resolve the issues.
Status: RESOLVED → REOPENED
Depends on: 385091
Resolution: FIXED → ---
Priority: -- → P2
Product: Core → MailNews Core
Rich, I'd like to pick up the latest changes from the directory/c-sdk HEAD for Thunderbird/SeaMonkey.

I'd like to use a static tag (as I was trying to do before). Do you want/are you likely to do a 6.0.5 release in the next week or two, or shall I just create a static tag for us to use until the next LDAP c-sdk release?
(In reply to comment #70)
> Rich, I'd like to pick up the latest changes from the directory/c-sdk HEAD for
> Thunderbird/SeaMonkey.
> 
> I'd like to use a static tag (as I was trying to do before). Do you want/are
> you likely to do a 6.0.5 release in the next week or two, or shall I just
> create a static tag for us to use until the next LDAP c-sdk release?

6.0.5 was tagged with the static tag LDAPCSDK_6_0_5_RTM
(In reply to comment #71)
> (In reply to comment #70)
> > I'd like to use a static tag (as I was trying to do before). Do you want/are
> > you likely to do a 6.0.5 release in the next week or two, or shall I just
> > create a static tag for us to use until the next LDAP c-sdk release?
> 
> 6.0.5 was tagged with the static tag LDAPCSDK_6_0_5_RTM

Ah, I guess the version update was forgotten then, because build.mk and mozldap.spec both still say 6.0.4.

So the question becomes what are your thoughts on a 6.0.6 release?
(In reply to comment #72)
> (In reply to comment #71)
> > (In reply to comment #70)
> > > I'd like to use a static tag (as I was trying to do before). Do you want/are
> > > you likely to do a 6.0.5 release in the next week or two, or shall I just
> > > create a static tag for us to use until the next LDAP c-sdk release?
> > 
> > 6.0.5 was tagged with the static tag LDAPCSDK_6_0_5_RTM
> 
> Ah, I guess the version update was forgotten then, because build.mk and
> mozldap.spec both still say 6.0.4.
> 
> So the question becomes what are your thoughts on a 6.0.6 release?

Done. LDAPCSDK_6_0_6_RTM (and date tag LDAPCSDK_6_0_6_RTM_20080911).  I've created a source tarball and uploaded to ftp://ftp.mozilla.org/pub/directory/c-sdk/releases/v6.0.6/src too - should be there whenever the refresh goes from stage to ftp.
(In reply to comment #73)
> Done. LDAPCSDK_6_0_6_RTM (and date tag LDAPCSDK_6_0_6_RTM_20080911).  I've
> created a source tarball and uploaded to
> ftp://ftp.mozilla.org/pub/directory/c-sdk/releases/v6.0.6/src too - should be
> there whenever the refresh goes from stage to ftp.

Excellent, thanks Rich.
This upgrades SeaMonkey/Thunderbird to the latest 6.0.6 version. This means we now track a static tag (as previously discussed is what we were aiming for), and gets the latest changes that haven't gone in on the client branch (see the two links in comment 69).

Whilst not major, I'd like to do this as I think it may simplify some of the release process for TB 3b1 (and even if it doesn't, we pick up a few more fixes anyway).
Attachment #338612 - Flags: superreview?(dmose)
Attachment #338612 - Flags: review?(dmose)
Comment on attachment 338612 [details] [diff] [review]
[checked in] Update Thunderbird and SeaMonkey to version 6.0.6

Sounds good; r+sr=dmose
Attachment #338612 - Flags: superreview?(dmose)
Attachment #338612 - Flags: superreview+
Attachment #338612 - Flags: review?(dmose)
Attachment #338612 - Flags: review+
Comment on attachment 338612 [details] [diff] [review]
[checked in] Update Thunderbird and SeaMonkey to version 6.0.6

Checked in, changeset id: 370:637edb398796
Attachment #338612 - Attachment description: Update Thunderbird and SeaMonkey to version 6.0.6 → [checked in] Update Thunderbird and SeaMonkey to version 6.0.6
Attachment #271367 - Attachment is obsolete: true
Status: REOPENED → RESOLVED
Closed: 12 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b1
You need to log in before you can comment on or make changes to this bug.