131849 - addressbook build system requires exporting internal files

Status: NEW

(MailNews Core :: Address Book, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

When addrbook/build attempts to build (at least on Windows and *nix), it depends
on every header in addrbook/src being exported to dist for no good reason.

Instead, it should just add 

LOCAL_INCLUDES += -I$(srcdir)/../src

or equivalent.

Comment 1

[Mark Banner (:standard8)]

Attachment: Patch (checked in)

This patch only exports the necessary header files to dist, in addition, a
couple of files are changed to support this. I've tested on linux with clobber
builds of thunderbird and suite and it appears to work fine. I can't see any
reason why the windows build should fail either because the headers that were
exported into dist  aren't required for any of the idl files.

Comment 2

[[e-mail]]

Comment on attachment 177305 [details] [diff] [review]
Patch (checked in)

Sorry, but although I'm sure this is good stuff I don't know the build system
well enough to review this.

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 177305 [details] [diff] [review]
Patch (checked in)

For reviewer's info: this is a build configuration change for address book
header files.

Comment 4

[Mark Banner (:standard8)]

Patch checked in by timeless. Leaving this open whilst checking all trees remain
green.

2005-03-14 09:30	timeless%mozdev.org 	mozilla/ mailnews/ import/ text/ src/
nsTextAddress.cpp 	1.44 	2/1
2005-03-14 09:30	timeless%mozdev.org 	mozilla/ mailnews/ build/ Makefile.in 
1.10 	1/0 
2005-03-14 09:30	timeless%mozdev.org 	mozilla/ mailnews/ addrbook/ src/
nsAbBaseCID.h 	1.6 	0/1 
2005-03-14 09:30	timeless%mozdev.org 	mozilla/ mailnews/ addrbook/ src/
Makefile.in 	1.55 	0/42 
2005-03-14 09:30	timeless%mozdev.org 	mozilla/ mailnews/ addrbook/ build/
Makefile.in 	1.46 	1/0 

Comment 5

[Mark Banner (:standard8)]

Attachment: Fix build bustage on Win & Mac (checked in)

This will hopefully fix the build bustage on windows and mac.

Comment 6

[Mark Banner (:standard8)]

Attachment: Second attempt to fix bustage on Windows (checked in)

Ok, second attempt to fix the bustage on windows - this time it's palm sync.
I've had to put a couple of the taken out headers back in as palmsync uses the
member variables of one of the non-exported classes :-(

Comment 7

[Mark Banner (:standard8)]

Attachment: Additional bustage fix (checked in)

This patch was the additional fixes supplied by timeless (thank you) for
finally fixing the windows bustage.

The nsAbPalmSync.cpp change in this patch was a temporary one to copy the
static function "PRTime2Seconds" from nsAddrDatabase.cpp to the former file.
This means we now have 4 definitions of this same function in mailnews.

Before I close this bug, I want to resolve the above PRTime2Seconds item, and
also see if the palm sync extension really needs the nsAbCardProperty.h and
nsAbMDBCardProperty.h headers - ideally it should run off the idl files like
the rest of mailnews.

Comment 8

[Mark Banner (:standard8)]

Attachment: Centralise PRTime2Seconds functions

This patch does a bit of tidy up to centralise the PRTime2Seconds functions
(and Seconds2PRTime function) into one central place in nsMsgUtils. To do this
without adding the minimal of extra requires to address book & palm sync, I've
re-arranged some of the includes.

As far as I can tell Palm Sync should work, but I can't get a Windows dev
environment to work, and I'd like to get this in.

Comment 9

[David :Bienvenu]

Comment on attachment 177892 [details] [diff] [review]
Centralise PRTime2Seconds functions

looks OK - I can try this patch against palmsync...

Comment 10

[Mark Banner (:standard8)]

Attachment: Centralise PRTime2Seconds funcs and fix palm sync compilation (checked in)

Updated previous patch that David tested for me with palm sync - missing item
in the requires for palm sync. Carrying forward his r.

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 178141 [details] [diff] [review]
Centralise PRTime2Seconds funcs and fix palm sync compilation (checked in)

Timeless checked this patch in.

2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ db/ msgdb/
src/ nsMsgHdr.cpp	  1.115   2/2	  Bug 131849 addressbook build system
requires exporting internal files patch by [e-mail]
r=bienvenu sr=neil
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ extensions/
palmsync/ src/ Makefile.in	 1.7	 2/0 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ extensions/
palmsync/ src/ nsAbPalmSync.cpp  1.26	 1/9 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ imap/ src/
nsImapProtocol.cpp	  1.601   1/9 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ addrbook/
src/ Makefile.in   1.57    1/0 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ addrbook/
src/ nsAddrDatabase.cpp    1.127   1/10 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ addrbook/
src/ nsAddrDatabase.h	   1.50    0/3 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ base/ src/
nsMsgSearchDBView.cpp	  1.44	  1/0 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ base/ util/
nsMsgUtils.cpp	 1.103	 29/0 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ base/ util/
nsMsgUtils.h	 1.48	 11/2 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ db/ msgdb/
public/ nsMsgDatabase.h   1.117   0/5 
2005-03-22 09:39	timeless%mozdev.org	mozilla/ mailnews/ db/ msgdb/
src/ nsMsgDatabase.cpp	  1.349   0/19

Comment 12

[Mark Banner (:standard8)]

Ok, the main parts of this patch are in, it would be nice to rework some of the
palm sync code such that address book doesn't have to export nsAbCardProperty.h
and nsAbMDBCardProperty.h - but that is for now waiting on some fairly major
code changes to nsIAbCard.idl in bug 279405.

Comment 13

[Mark Banner (:standard8)]

Attachment: Remove nsVCard.h from exports (checked in)

Simple patch so we don't export nsVCard.h. I can't see anywhere outside of
address book where this file is used, and seamonkey & tb compile ok with this
patch on linux. We still need to export nsVCardObj.h at the current time (hence
I haven't removed it).

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 187230 [details] [diff] [review]
Remove nsVCard.h from exports (checked in)

r+sr=dmose

Comment 15

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 187230 [details] [diff] [review]
Remove nsVCard.h from exports (checked in)

Be sure you've tested this patch against a clobber tree, or at the very least
after explicitly removing any existing links/copies of nsVCard.h from dist.

Comment 16

[Mark Banner (:standard8)]

Comment on attachment 187230 [details] [diff] [review]
Remove nsVCard.h from exports (checked in)

Tested patch against clobber builds for tb & suite and no problems found.

Requesting approval, this patch stops address book exporting a header file that
it doesn't need to. No functional changes.

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 187230 [details] [diff] [review]
Remove nsVCard.h from exports (checked in)

Checking in mailnews/addrbook/public/nsIMsgVCardService.idl;
/cvsroot/mozilla/mailnews/addrbook/public/nsIMsgVCardService.idl,v  <-- 
nsIMsgVCardService.idl
new revision: 1.3; previous revision: 1.2
done
Checking in mailnews/addrbook/src/Makefile.in;
/cvsroot/mozilla/mailnews/addrbook/src/Makefile.in,v  <--  Makefile.in
new revision: 1.60; previous revision: 1.59
done
Checking in mailnews/addrbook/src/nsMsgVCardService.cpp;
/cvsroot/mozilla/mailnews/addrbook/src/nsMsgVCardService.cpp,v	<-- 
nsMsgVCardService.cpp
new revision: 1.5; previous revision: 1.4
done

Comment 18

[Mark Banner (:standard8)]

Attachment: Start removing nsDirPrefs.h from public includes (checked in).

This patch starts to remove nsDirPrefs.h from the public includes, bug 323181 should do the rest of removing it from the exports.

Comment 19

[Mark Banner (:standard8)]

Comment on attachment 208282 [details] [diff] [review]
Start removing nsDirPrefs.h from public includes (checked in).

This patch checked in (as are all the others so far on this bug).

Comment 20

[David :Bienvenu]

Attachment: fix palmsync build bustage (checked in)

this gets palm sync building again - I'll check this in since it fixes build bustage.

Comment 21

[Mark Banner (:standard8)]

Attachment: Follow-up patch for palmsync build bustage fix (checked in)

This patch moves the added #defines to a central location in nsIAbDirectory. I don't see there being a problem with them there and they'd probably need to be moved eventually anyway.

Comment 22

[Mark Banner (:standard8)]

The only files that are left are the ones below:

nsAbCardProperty.h nsAbMDBCardProperty.h nsVCardObj.h

nsAb*CardProperty need palmsync to stop requiring the use of them. nsVCardObj.h needs a vcard rewrite.

I'm reassigning this to the default owner as I won't be working on either of these in the near future, but someone else may want to.