706186 - xulrunner-stub.exe depends on mozutils.dll

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Dave Townsend [:mossop]]

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.

Comment 1

[Dave Townsend [:mossop]]

As of XULRunner 9 it now depends on mozutils.dll

Comment 3

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This forces static libraries and doesn't link against mozglue, mozutils etc for the stub.

Comment 4

[Mike Hommey [:glandium]]

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 =

Comment 5

[Dave Townsend [:mossop]]

Attachment: 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.

Comment 6

[Dave Townsend [:mossop]]

Note I've tested this new patch on Windows and Linux, not on OSX yet.

Comment 7

[Mike Hommey [:glandium]]

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.

Comment 8

[Dave Townsend [:mossop]]

Attachment: 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.

Comment 9

[Mike Hommey [:glandium]]

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?

Comment 10

[Dave Townsend [:mossop]]

Attachment: patch rev 4

File size before is 17920, after is 98816.

Comment 11

[Mike Hommey [:glandium]]

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.

Comment 12

[Dave Townsend [:mossop]]

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

Comment 13

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/f622f39e4296

Comment 14

[Dave Townsend [:mossop]]

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

Comment 15

[Alex Keybl [:akeybl]]

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

[Triage Comment]
NPOTB for Firefox, so approving for Aurora 12 and Beta 11.

Comment 16

[Dave Townsend [:mossop]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5d66efa5b59b
https://hg.mozilla.org/releases/mozilla-beta/rev/7b1df36b517c