245430 - Investigate static build version of thunderbird

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Scott MacGregor]

tracking bug for various patches to support static builds

Comment 1

[Scott MacGregor]

Attachment: patch to at least get a dist\bin static build

ignores issues with packaging in mail\config

these changes at least get the build off the ground for windows

Comment 2

[Scott MacGregor]

Attachment: patch to help with OS X static builds

Comment 3

[Scott MacGregor]

Attachment: work in progress patch

this larger patch is still a work in progress and trys to remove the
dist\thunderbird packaging step.

Comment 4

[Scott MacGregor]

*** Bug 226800 has been marked as a duplicate of this bug. ***

Comment 5

[Scott MacGregor]

Attachment: updated work in progress patch

This eliminates the dist\thunderbird step for windows. 

mail\config only does the chrome repackaging

mail\installer\Makefile.in zips up the contents of dist\bin and removes files
we d on't want packaged that were previously not getting copied to
dist\thunderbird.

Comment 6

[Scott MacGregor]

Attachment: make the mail biff indicator work with static builds

Comment 7

[Scott MacGregor]

Attachment: patch to make sure the application icon doesn't use the biff icon

my previous patch (150060) got the mail biff indicator working but broke the
application icon associated with the executable. This patch addresses that.

Still one remaining problem: when you mouse over the biff indicator in the
system tray the icon is now disappearing. I don't know why that is yet.

Comment 8

[Scott MacGregor]

I think you also need the patch in

http://bugzilla.mozilla.org/show_bug.cgi?id=239835

to fix some weird issues with the mail biff icon and static builds. 

Comment 9

[Scott MacGregor]

*** Bug 245892 has been marked as a duplicate of this bug. ***

Comment 10

[Scott MacGregor]

Attachment: another attempt to fix mail biff indicator in the system tray

this patch restores the old mail biff resource ID of 101. It adds another
application icon that has a lower name than the biff icon ID of 101 because
windows goes in alphabetical order when picking the ICO from a resource to use
as the application icon.

Comment 11

[Scott MacGregor]

Attachment: try to fix OS X static build

Comment 12

[Scott MacGregor]

Attachment: fix static build crash on OS X. Don't use a global static string object

Comment 13

[Scott MacGregor]

Attachment: fix static build crash on OS X. Don't use global static string objects

Comment 14

[Scott MacGregor]

Comment on attachment 150467 [details] [diff] [review]
fix static build crash on OS X. Don't use global static string objects

On OS X, global static string objects crash if you are running a static build.
This kept Thunderbird static builds from starting up. 

So far I've only found this one occurrence. There may be more lurking out
there.

I added some extra nsMemory::Free calls before each assignment into
gDefaultCharacterSet to protect against any leaks.

Comment 15

[Scott MacGregor]

Attachment: another static global string

Comment 16

[Scott MacGregor]

Comment on attachment 150479 [details] [diff] [review]
another static global string

I fixed this instance by making it a regular property on the protocol instance
and not a static property. I moved the pref code to read in the pref from
InitGlobals to the IMAP ctor. Feel free to object if you don't like that
approach David.

Comment 17

[Scott MacGregor]

Attachment: wallet change

I need to test this wallet patch out more before landing on the trunk

Comment 18

[David :Bienvenu]

Comment on attachment 150479 [details] [diff] [review]
another static global string

sr=bienvenu

Comment 19

[Scott MacGregor]

there is no remaining work to be done here on the branch. Just work to land all
these patches on the trunk. 

Comment 20

[Bill Gianopoulos [:WG9s]]

This change appears to be the cause of the redness in the Linux Thunderbird
tinderbox build.  It is failing saying it can't find the thunderbird binary in
the dist directory.

Comment 21

[Scott MacGregor]

*** Bug 247571 has been marked as a duplicate of this bug. ***

Comment 22

[Haris Karahristianidis]

Still no white folder in tray for new mail... :-(

Comment 23

[Hussam Al-Tayeb]

(In reply to comment #22)
> Still no white folder in tray for new mail... :-(
It only shows an empty space.

Comment 24

[Haris Karahristianidis]

(In reply to comment #23)
> (In reply to comment #22)
> > Still no white folder in tray for new mail... :-(
> It only shows an empty space.
> 

Yes, an annoying empty space...

Comment 25

[Scott MacGregor]

I don't think the missing biff indicator has anything to do with the static
build changes on windows anymore. I now see the problem in non static debug
builds. I think it has to do with the way processes are spun up with the new
profile manager.  So I've spun off a separate bug, Bug #245430

Comment 26

[Haris Karahristianidis]

(In reply to comment #25)
> I don't think the missing biff indicator has anything to do with the static
> build changes on windows anymore. I now see the problem in non static debug
> builds. I think it has to do with the way processes are spun up with the new
> profile manager.  So I've spun off a separate bug, Bug #245430

Is there a seperate bug for biff indicator?

Comment 27

[Scott MacGregor]

this is actually all checked in on the branch and the trunk except for the
wallet change. I need to get that reviewed so it can land on the trunk. 

Comment 28

[Chris P. Ross]

This was noted as fixed on the branch, and mostly in the trunk.  It didn't sound
like it would take 3 more releases to fix it.  Why was this pushed out past 0.9?  

Comment 29

[Scott MacGregor]

it is fixed on the branch and the trunk. The only thing that isn't a static
build is the OS X build because we need some wallet changes I haven't had time
to get into the trunk yet. 

Comment 30

[Chris P. Ross]

We should at least update the summary (or OS), then, since the problem has been
fixed for all OSes but one.  Make it look less like this is a big problem that's
being delayed that long...

Comment 31

[Scott MacGregor]

Comment on attachment 150482 [details] [diff] [review]
wallet change

Philip did some string work in wallet recently. I'll tag him for a reviewer for
this patch :)

In order to use wallet in a static build on the mac, we can't have global
nsAutoStrings defined anywhere.

Comment 32

[Philip K. Warren]

Comment on attachment 150482 [details] [diff] [review]
wallet change

>@@ -2759,7 +2759,7 @@
>   }
> 
>   buffer.Append(BREAK);
>-  buffer += wallet_url.get();
>+  buffer += wallet_url;

Can wallet_url ever be unitialized or null here?

>@@ -3314,6 +3314,8 @@
>   if (fillins.Length() == 0) { /* user pressed CANCEL */
>     wallet_ReleasePrefillElementList(wallet_list);
>     wallet_list = nsnull;
>+    nsMemory::Free(wallet_url);
>+    wallet_url = NULL;

Maybe change this to nsnull when you check it in to more closely match the rest
of the code.

Be careful with the wallet code - it is very fragile =).

Comment 33

[Scott MacGregor]

Comment on attachment 150482 [details] [diff] [review]
wallet change

remove global autostring so we can turn on static builds for Thunderbird Mac OS
X.

I'll change:
wallet_url = NULL;
to 
wallet_url = nsnull;

before checking in

Comment 34

[David :Bienvenu]

Comment on attachment 150482 [details] [diff] [review]
wallet change

-    wallet_url = urlName;
+    wallet_url = ToNewUnicode(urlName);

w/o seeing the surrounding code, this isn't going to cause a leak, is it? is it
possible that wallet_url has a value here that we're crunching, since it's a
global?

Comment 35

[Scott MacGregor]

Attachment: improved wallet change

addresses David's concern by calling nsMemory::Free on wallet_url before we
call ToNewUnicode.

Comment 36

[Scott MacGregor]

the wallet change has been checked in. Once we fix Bug #286742, then we can turn
on static builds for Mac OS X.

Comment 37

[Scott MacGregor]

mac builds are now static on the trunk

Comment 38

[David :Bienvenu]

Comment on attachment 150482 [details] [diff] [review]
wallet change

clearing request