76788 - remove redundant global symbols from address book

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect, P3)

Description

[Chris Waterson]

This stuff is checked in on STATIC_BUILD_20010418_BRANCH, but needs r=.

Comment 1

[Chris Waterson]

Taking, since I have diffs.

Comment 2

[Chris Waterson]

Attachment: remove redundant symbols in absync, etc.

Comment 3

[Chris Waterson]

candace, could you r=? thanks.

Comment 4

[Chris Waterson]

Attachment: oops, wrong patch

Comment 5

[chuang]

r=chuang.

Comment 6

[Chris Waterson]

sr=waterson, ship it!

Comment 7

[Chris Waterson]

(these changes are on the STATIC_BUILD_20010418_BRANCH.)

Comment 8

[dprice (gone)]

-> kandrot

Comment 9

[Edward Kandrot]

Part of tonight's checkin (part 3 of 3 in my part of checking in Ron's changes).

Comment 10

[chris hofmann]

dribble this in as you can get to it.
not critical for the 0.9.1 betas that I know of but we want to continue with
progress.
moving the target milestone to reduce the size of the list.
check it in in the next week if its ready

Comment 11

[Edward Kandrot]

Checked in the changes.

Comment 12

[(not reading, please use [e-mail] instead)]

this patch doesn't build on win32.

forgive my ignorance here, but if we make kFirstNameColumn extern, where will 
it find it?  nsAddrDatabase.cpp?  that's not in the same module.

Comment 13

[(not reading, please use [e-mail] instead)]

did this get tested on linux?  if I'm right, it would have died on loading the 
absync module due to the unresolved symbols.  it would have built fine, though.

kandrot, can you back this out?

Comment 14

[(not reading, please use [e-mail] instead)]

to be fair, you'd probably need a commercial build to test this.

chuang?

Comment 15

[Cathleen]

We got an optimized linux static build from the brach.  

The tests we ran are only limited to startup and page-load performance.
There was no intensive QA blessings on every features in the static build. 

We definitely need to test this thing throughly, identify bugs and fix them. If 
we do decide to ship it.

Comment 16

[Cathleen]

reopen this bug...

address book changes was backed out of trunk due to build bustage on win32 a 
while back.

sspitzer or chuang, we will need your help to get address book changes correctly 
so it will build on windows. :-)  we're cutting a new static build branch right 
now;  will inform you guys when we're ready.

-thanks!  

Comment 17

[Edward Kandrot]

Is there a set of changes to this patch that will fix the problems we had when
last I checked in?  Who is the person to contact who can make the needed
changes.  I do not know what the code does, or I would.  Thanks.

Comment 18

[(not reading, please use [e-mail] instead)]

there hasn't been a patch (yet) that will work for both the static branch and
the tip.

the problem is that these strings have to live in two different modules on the
trunk and then within the same module on the static branch.

can't we just make them static where they are used in both places, so that when
linking the two .a libraries together, there won't be duplicately defined
symbols?  there will be some extra bloat (the strings will be defined twice, but
statically only), but at least the same fix will work for the tip as well as the
static branch.

I can work with kandrot soon (tomorrow?) on this.  

I can help 

Comment 19

[chuang]

Try the two patches,  move the constants into idl.  I tried it on the trunk and
it is working.  Hope it will work on the static build. 

Comment 20

[chuang]

Attachment: patch againt trunk for addrbook

Comment 21

[chuang]

Attachment: patch against trunk for absync

Comment 22

[chuang]

The patches are against trunk.  It moves the constants into nsIAddrDatabase.idl
and remove all extern const statements from addrbook and absync.

Comment 23

[Cathleen]

Thanks you Candice!

I manually applied the patches to our static branch and they resolve the build
bustage for dynamic build.  I will go ahead check in your patches into to our 
branch.  They will land when we get the branch landed on the trunk!

Thanks you for whipping out the fixes so quickly!  :-)

Comment 24

[Cathleen]

Attachment: diff of applying candice's trunk patches to addrbook and absync against static branch

Comment 25

[Cathleen]

patch landed to static branch

Comment 26

[Chris Waterson]

I'm still seeing some duplicate symbols...

 multiple definition of `kAddressCharSetColumn'
 multiple definition of `kMailListName'
 multiple definition of `kMailListNickName'
 multiple definition of `kMailListDescription'
 multiple definition of `kMailListTotalAddresses'

Comment 27

[Cathleen]

Attachment: new patch, this should fix those conflict symbols. candice, can you review?

Comment 28

[chuang]

r = chuang. Looks good.

Comment 29

[cls]

When are these going back into the trunk?

Comment 30

[Cathleen]

this is fixed by waterson landing the static branch today.

Comment 31

[[e-mail] (gone - use [e-mail] instead)]

Fixed using code verification through LXR.

Verified.