Closed Bug 76788 Opened 19 years ago Closed 19 years ago

remove redundant global symbols from address book

Categories

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

x86
Linux
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: waterson, Assigned: kandrot)

References

Details

(Keywords: memory-footprint)

Attachments

(6 files)

This stuff is checked in on STATIC_BUILD_20010418_BRANCH, but needs r=.
Taking, since I have diffs.
Assignee: chuang → waterson
Priority: -- → P3
Target Milestone: --- → mozilla0.9.1
candace, could you r=? thanks.
Blocks: 46775
r=chuang.
Status: NEW → ASSIGNED
sr=waterson, ship it!
Assignee: waterson → dprice
Status: ASSIGNED → NEW
(these changes are on the STATIC_BUILD_20010418_BRANCH.)
-> kandrot
Assignee: dprice → kandrot
Part of tonight's checkin (part 3 of 3 in my part of checking in Ron's changes).
Status: NEW → ASSIGNED
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
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Checked in the changes.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
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.
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?
to be fair, you'd probably need a commercial build to test this.

chuang?
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.
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!  
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
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 

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. 
The patches are against trunk.  It moves the constants into nsIAddrDatabase.idl
and remove all extern const statements from addrbook and absync.
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!  :-)

patch landed to static branch
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'
r = chuang. Looks good.
When are these going back into the trunk?
this is fixed by waterson landing the static branch today.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
QA Contact: esther → stephend
Fixed using code verification through LXR.

Verified.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.