Closed Bug 1060848 Opened 6 years ago Closed 6 years ago

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

Categories

(SeaMonkey :: Build Config, defect)

x86
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.32

People

(Reporter: philip.chee, Assigned: iann_bugzilla)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ 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
What is the marketshare of XP SP2 on SeaMonkey? SeaMonkey has a more technical audience than Firefox.
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Attached patch Port changes to app/moz.build (obsolete) — Splinter Review
The patches from bug 882968 and bug 1055867 would need to be applied before applying this one.
Attachment #8481903 - Flags: review?(bugspam.Callek)
(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 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.]
Depends on: 1061338
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.
Flags: needinfo?(iann_bugzilla)
Revised patch as per comments by Neil
Should apply cleanly now patch queue checked in.
Attachment #8481903 - Attachment is obsolete: true
Attachment #8481903 - Flags: review?(bugspam.Callek)
Attachment #8483776 - Flags: review?(bugspam.Callek)
Attachment #8483776 - Flags: feedback?(neil)
Flags: needinfo?(iann_bugzilla)
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.
Attachment #8483776 - Flags: feedback?(neil)
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.
Attachment #8483776 - Flags: feedback?(neil)
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.
Attachment #8483776 - Flags: feedback?(neil) → feedback+
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.
Attachment #8483776 - Flags: review?(bugspam.Callek) → review+
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 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
Attachment #8483776 - Attachment description: Revised port changes to app/moz.build → Revised port changes to app/moz.build [Checked in: Comment 12]
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.32
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).
You need to log in before you can comment on or make changes to this bug.