The default bug view has changed. See this FAQ.

xulrunner-stub.exe depends on mozutils.dll

RESOLVED FIXED in mozilla13

Status

Toolkit Graveyard
XULRunner
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

Trunk
mozilla13
x86_64
Windows 7
Dependency tree / graph

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
At some point (since Gecko 8 at least) xulrunner-stub.exe started depending on mozcrt19.dll. This means you can't just copy the stub to your app dir to run the app. Probably it works when copying the dll too though I didn't yet test that. Ideally we would just remove this dependency though.
(Assignee)

Comment 1

5 years ago
As of XULRunner 9 it now depends on mozutils.dll
Summary: xulrunner-stub.exe depends on mozcrt19.dll → xulrunner-stub.exe depends on mozutils.dll
(Assignee)

Updated

5 years ago
Assignee: nobody → dtownsend
Duplicate of this bug: 723085
(Assignee)

Comment 3

5 years ago
Created attachment 601743 [details] [diff] [review]
patch rev 1

This forces static libraries and doesn't link against mozglue, mozutils etc for the stub.
Attachment #601743 - Flags: review?(benjamin)
(Assignee)

Updated

5 years ago
Blocks: 722810
Comment on attachment 601743 [details] [diff] [review]
patch rev 1

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

::: xulrunner/stub/Makefile.in
@@ +43,5 @@
>  
>  include $(DEPTH)/config/autoconf.mk
>  
>  MODULE    = xulrunner
> +USE_STATIC_LIBS = 1

I'm not convinced you need this.

@@ +92,5 @@
>  endif
>  
>  include $(topsrcdir)/config/config.mk
>  
> +MOZ_UTILS_PROGRAM_LDFLAGS =

This effectively removes jemalloc from the stub on linux. You probably want MOZ_UTILS_LDFLAGS =
Attachment #601743 - Flags: review?(benjamin) → review-
(Assignee)

Comment 5

5 years ago
Created attachment 601802 [details] [diff] [review]
patch rev 2

Should have checked that the patch I wrote a few months ago was still the right fix, it almost was...

(In reply to Mike Hommey [:glandium] from comment #4)
> Comment on attachment 601743 [details] [diff] [review]
> patch rev 1
> 
> Review of attachment 601743 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: xulrunner/stub/Makefile.in
> @@ +43,5 @@
> >  
> >  include $(DEPTH)/config/autoconf.mk
> >  
> >  MODULE    = xulrunner
> > +USE_STATIC_LIBS = 1
> 
> I'm not convinced you need this.

Without this the stub includes a dependency on mozglue (http://mxr.mozilla.org/mozilla-central/source/config/config.mk#253) and on the msvcr dll (http://mxr.mozilla.org/mozilla-central/source/config/config.mk#507), neither of which are wanted for the stub.

> @@ +92,5 @@
> >  endif
> >  
> >  include $(topsrcdir)/config/config.mk
> >  
> > +MOZ_UTILS_PROGRAM_LDFLAGS =
> 
> This effectively removes jemalloc from the stub on linux. You probably want
> MOZ_UTILS_LDFLAGS =

Actually turns out this isn't necessary anymore, or I was mistaken about it being necessary in the first place.
Attachment #601743 - Attachment is obsolete: true
Attachment #601802 - Flags: review?(mh+mozilla)
(Assignee)

Comment 6

5 years ago
Note I've tested this new patch on Windows and Linux, not on OSX yet.
Comment on attachment 601802 [details] [diff] [review]
patch rev 2

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

::: xulrunner/stub/Makefile.in
@@ +43,5 @@
>  
>  include $(DEPTH)/config/autoconf.mk
>  
>  MODULE    = xulrunner
> +USE_STATIC_LIBS = 1

In practice, what this does is "MOZ_GLUE_LDFLAGS=" on windows. Not on mac, where you'd want the same thing. So I think the right fix would be MOZ_GLUE_LDFLAGS= instead.
Attachment #601802 - Flags: review?(mh+mozilla) → review-
(Assignee)

Comment 8

5 years ago
Created attachment 602134 [details] [diff] [review]
patch rev 3

Ok, last try hopefully. Verified that this removes all non-system dependencies on linux, mac and windows.

USE_STATIC_LIBS to drop the msvc dependencies on windows.
MOZ_GLUE_LDFLAGS to drop the mozglue dependencies on windows and mac.

Linux seems ok by itself for some reason.
Attachment #601802 - Attachment is obsolete: true
Attachment #602134 - Flags: review?(mh+mozilla)
Comment on attachment 602134 [details] [diff] [review]
patch rev 3

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

::: xulrunner/stub/Makefile.in
@@ +44,5 @@
>  include $(DEPTH)/config/autoconf.mk
>  
>  MODULE    = xulrunner
> +USE_STATIC_LIBS = 1
> +MOZ_GLUE_LDFLAGS =

Please put a comment as to why these are there.
What is the file size before and after, on windows?
(Assignee)

Comment 10

5 years ago
Created attachment 602190 [details] [diff] [review]
patch rev 4

File size before is 17920, after is 98816.
Attachment #602134 - Attachment is obsolete: true
Attachment #602134 - Flags: review?(mh+mozilla)
Attachment #602190 - Flags: review?(mh+mozilla)
Comment on attachment 602190 [details] [diff] [review]
patch rev 4

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

> File size before is 17920, after is 98816.

Not too bad, I'd say. I wonder how small it would get if the stub was rewritten in C (which it mostly is already). But I don't think it matters for this bug, except if bsmedberg objects to the jump in size.
Attachment #602190 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 12

5 years ago
Certainly cheaper than also having to copy the ~160kb of msvc* dlls alongside the stub in your app.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f622f39e4296
https://hg.mozilla.org/mozilla-central/rev/f622f39e4296
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
(Assignee)

Comment 14

5 years ago
Comment on attachment 602190 [details] [diff] [review]
patch rev 4

[Approval Request Comment]
Regression caused by (bug #): Unknown
User impact if declined: XULRunner apps have to use an older or nightly SDK to get a usable stub exe
Testing completed (on m-c, etc.): On nightly
Risk to taking this patch (and alternatives if risky): No risk to Firefox, isn't part of the regular build.
String changes made by this patch: None
Attachment #602190 - Flags: approval-mozilla-beta?
Attachment #602190 - Flags: approval-mozilla-aurora?
Comment on attachment 602190 [details] [diff] [review]
patch rev 4

[Triage Comment]
NPOTB for Firefox, so approving for Aurora 12 and Beta 11.
Attachment #602190 - Flags: approval-mozilla-beta?
Attachment #602190 - Flags: approval-mozilla-beta+
Attachment #602190 - Flags: approval-mozilla-aurora?
Attachment #602190 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/5d66efa5b59b
https://hg.mozilla.org/releases/mozilla-beta/rev/7b1df36b517c
status-firefox11: --- → fixed
status-firefox12: --- → fixed
Whiteboard: [qa-]
Depends on: 741456
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.