1060848 - Static-link the CRT into SeaMonkey executable (/suite/app/moz.build). Port Bug 1023941 Part 1

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Philip Chee]

+++ This bug was initially created as a clone of Bug #1023941 +++

(from Bug 1023941 comment #0)

> In order to be able to support Windows XP SP2 with Visual C++ 2013, we need
> to link statically to the CRT in firefox.exe and mozglue.dll but dynamically
> to everything else.
> 
> Is this something that the build system already supports?  If not, can you
> please give us a rough estimate on how much work that's going to be and how
> it would work?
> 
> Thanks!

Looks like we need to port:
Part 1: Static-link the CRT into firefox.exe.
https://hg.mozilla.org/mozilla-central/rev/baa3f852133b

Comment 1

[Yuhong Bao]

What is the marketshare of XP SP2 on SeaMonkey? SeaMonkey has a more technical audience than Firefox.

Comment 2

[Ian Neal]

Attachment: Port changes to app/moz.build

The patches from bug 882968 and bug 1055867 would need to be applied before applying this one.

Comment 3

[[e-mail]]

(In reply to Yuhong Bao from comment #1)
> What is the marketshare of XP SP2 on SeaMonkey? SeaMonkey has a more
> technical audience than Firefox.

Not relevant, our CI debug builds will crash if this isn't ported.

Comment 4

[[e-mail]]

Comment on attachment 8481903 [details] [diff] [review]
Port changes to app/moz.build

>+    DELAYLOAD_DLLS += [
>+        'mozglue.dll',
>     ]
> 
>+    USE_STATIC_LIBS = True
[When I had to get my debug build working I put these bits in the WINNT section as it didn't seem to make sense to delay load mozglue.dll on other platforms.]

Comment 5

[Philip Chee]

Comment on attachment 8481903 [details] [diff] [review]
Port changes to app/moz.build

>      if CONFIG['OS_ARCH'] == 'WINNT':
>          RCINCLUDE = 'splash.rc'
>          DEFINES['MOZ_SUITE'] = True
> +        USE_LIBS += [
> +            'mozglue',
> +            'xpcomglue_staticruntime',
> +        ]
> +    else:
> +        USE_LIBS += [
> +            'xpcomglue',
> +        ]
FYI This patch doesn't apply cleanly.

Comment 6

[Ian Neal]

Attachment: Revised port changes to app/moz.build [Checked in: Comment 12]

Revised patch as per comments by Neil
Should apply cleanly now patch queue checked in.

Comment 7

[[e-mail]]

Comment on attachment 8483776 [details] [diff] [review]
Revised port changes to app/moz.build [Checked in: Comment 12]

Are there some patches this depends on? I thought it was just because my tree wasn't up-to-date but I can't get this to apply.

Comment 8

[Ian Neal]

Comment on attachment 8483776 [details] [diff] [review]
Revised port changes to app/moz.build [Checked in: Comment 12]

I've double checked, this patch applies fine on top of current trunk.

Comment 9

[[e-mail]]

Ah, my fault, somehow my local changes avoided giving a merge conflict when I updated so I hadn't realised that the file had actually been modified.

Comment 10

[Justin Wood (:Callek)]

Comment on attachment 8483776 [details] [diff] [review]
Revised port changes to app/moz.build [Checked in: Comment 12]

Review of attachment 8483776 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/app/moz.build
@@ +26,5 @@
> +        USE_LIBS += [
> +            'mozglue',
> +            'xpcomglue_staticruntime',
> +        ]
> +        USE_STATIC_LIBS = True

DELAYLOAD_DLLS and USE_STATIC_LIBS should be outside this windows-only block, since they apply to other platforms too, afaict (as in comparing with Firefox's same file, today).

the USE_LIBS+= pieces do belong on both sides of the =='WINNT' as you have them though.

Comment 11

[(Away)]

DELAYLOAD_DLLS and USE_STATIC_LIBS only affect Windows builds, so it could go inside WINNT or not. I'm guilty myself of using them inconsistently.

Comment 12

[Ian Neal]

Comment on attachment 8483776 [details] [diff] [review]
Revised port changes to app/moz.build [Checked in: Comment 12]

http://hg.mozilla.org/comm-central/rev/9fb82ca1eaa0
Checked in as is taking on board comment 11

Comment 13

[Frank Wein [:mcsmurf]]

BTW: One also needs to apply this patch on current comm-beta to avoid crash-on-startup error for debug builds (not sure if this really needs to be ported, there aren't many debug users).