Last Comment Bug 678161 - msvc*.dll missing when building with new style jemalloc
: msvc*.dll missing when building with new style jemalloc
Status: RESOLVED FIXED
fixed-in-bs
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Windows 7
: -- normal (vote)
: mozilla9
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on: 681893
Blocks: 652263 678195 694371
  Show dependency treegraph
 
Reported: 2011-08-11 05:16 PDT by Mike Hommey [:glandium]
Modified: 2012-03-14 13:35 PDT (History)
4 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Also install msvc*.dll when using new style jemalloc (765 bytes, patch)
2011-08-11 05:26 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Also install msvc*.dll when using new style jemalloc (3.50 KB, patch)
2011-08-11 07:56 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Also install msvc*.dll when using new style jemalloc (5.29 KB, patch)
2011-08-18 23:22 PDT, Mike Hommey [:glandium]
khuey: review+
Details | Diff | Review
Also install msvc*.dll when using new style jemalloc [Checked in: Comment 12] (5.91 KB, patch)
2011-08-23 06:43 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review

Description Mike Hommey [:glandium] 2011-08-11 05:16:13 PDT
When building with VC10, the new style jemalloc is used, which requires the CRT to be copied over.
http://mxr.mozilla.org/mozilla-central/source/build/win32/Makefile.in#65 prevents that from happening
Comment 1 Mike Hommey [:glandium] 2011-08-11 05:26:12 PDT
Created attachment 552340 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc
Comment 2 Mike Hommey [:glandium] 2011-08-11 05:33:12 PDT
Comment on attachment 552340 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc

Need some more
Comment 3 Mike Hommey [:glandium] 2011-08-11 07:56:37 PDT
Created attachment 552377 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc

This takes care of installing the files in the package, and makes the mobile manifest look like the browser one.
Comment 4 Mike Hommey [:glandium] 2011-08-11 09:23:52 PDT
Comment on attachment 552377 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc

the CRT is also missing from the js shell zip.
Comment 5 Mike Hommey [:glandium] 2011-08-18 23:22:57 PDT
Created attachment 554318 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc

Not sure if it's fine to let zip warn about the missing files when not building with WIN32_REDIST_DIR set.
Comment 6 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2011-08-23 06:13:50 PDT
Comment on attachment 554318 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc

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

A couple comments

::: browser/installer/removed-files.in
@@ +1277,5 @@
>      msvcm80.dll
>      msvcp80.dll
>      msvcr80.dll
>    #else
>      mozcrt19.dll

Shouldn't mozcpp19.dll be here too?

::: mobile/installer/package-manifest.in
@@ +63,5 @@
> +@BINPATH@/mozcpp19.dll
> +#else
> +#ifdef MOZ_MEMORY
> +@BINPATH@/jemalloc.dll
> +#endif

Does mobile not need adjustment in removed-files.in?

::: toolkit/mozapps/installer/packager.mk
@@ +106,5 @@
> +endif
> +ifeq ($(_MSC_VER),1500)
> +JSSHELL_BINS += $(DIST)/bin/msvcr100.dll
> +endif
> +endif

Really sucks that we have to duplicate all this gunk here :-(
Comment 7 Mike Hommey [:glandium] 2011-08-23 06:17:26 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6)
> Comment on attachment 554318 [details] [diff] [review]
> Also install msvc*.dll when using new style jemalloc
> 
> Review of attachment 554318 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> A couple comments
> 
> ::: browser/installer/removed-files.in
> @@ +1277,5 @@
> >      msvcm80.dll
> >      msvcp80.dll
> >      msvcr80.dll
> >    #else
> >      mozcrt19.dll
> 
> Shouldn't mozcpp19.dll be here too?

mozcpp19.dll isn't there anymore. New style jemalloc builds a mozcrt.lib that helps linking jemalloc with the right hacked static portion of the crt, but that's all. mozcpp19.dll is replaced by jemalloc.dll+the CRT dlls

> Does mobile not need adjustment in removed-files.in?

I was wondering... its removed-files.in is mostly empty.

> ::: toolkit/mozapps/installer/packager.mk
> @@ +106,5 @@
> > +endif
> > +ifeq ($(_MSC_VER),1500)
> > +JSSHELL_BINS += $(DIST)/bin/msvcr100.dll
> > +endif
> > +endif
> 
> Really sucks that we have to duplicate all this gunk here :-(

An alternative could be that we set a variable with the list of crt files somewhere in config/config.mk and use that variable to fill the rest.
Comment 8 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2011-08-23 06:20:00 PDT
(In reply to Mike Hommey [:glandium] from comment #7)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6)
> > Comment on attachment 554318 [details] [diff] [review]
> > Also install msvc*.dll when using new style jemalloc
> > 
> > Review of attachment 554318 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > A couple comments
> > 
> > ::: browser/installer/removed-files.in
> > @@ +1277,5 @@
> > >      msvcm80.dll
> > >      msvcp80.dll
> > >      msvcr80.dll
> > >    #else
> > >      mozcrt19.dll
> > 
> > Shouldn't mozcpp19.dll be here too?
> 
> mozcpp19.dll isn't there anymore. New style jemalloc builds a mozcrt.lib
> that helps linking jemalloc with the right hacked static portion of the crt,
> but that's all. mozcpp19.dll is replaced by jemalloc.dll+the CRT dlls

Right, I know that (I wrote that code :-) ).  I'm asking why mozcpp19.dll isn't in removed-files.in.

> > Does mobile not need adjustment in removed-files.in?
> 
> I was wondering... its removed-files.in is mostly empty.

I think that's mostly because mobile hasn't been around that long (there's stuff in Firefox's removed-file.in from the ancient days).
 
> > ::: toolkit/mozapps/installer/packager.mk
> > @@ +106,5 @@
> > > +endif
> > > +ifeq ($(_MSC_VER),1500)
> > > +JSSHELL_BINS += $(DIST)/bin/msvcr100.dll
> > > +endif
> > > +endif
> > 
> > Really sucks that we have to duplicate all this gunk here :-(
> 
> An alternative could be that we set a variable with the list of crt files
> somewhere in config/config.mk and use that variable to fill the rest.

Perhaps.  I wouldn't block this on figuring out what to do here though.
Comment 9 Mike Hommey [:glandium] 2011-08-23 06:43:01 PDT
Created attachment 555090 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc
[Checked in: Comment 12]

As landing
Comment 10 Mike Hommey [:glandium] 2011-08-23 06:45:52 PDT
http://hg.mozilla.org/projects/build-system/rev/9b6e97e91525
Comment 11 Takeshi Ichimaru(a.k.a. Ayakawa) 2011-08-24 16:08:16 PDT
on toolkit/mozapps/installer/packager.mk,

+ifeq ($(_MSC_VER),1500)
+JSSHELL_BINS += $(DIST)/bin/msvcr100.dll
+endif

but,

+ifeq ($(_MSC_VER),1600)
+JSSHELL_BINS += $(DIST)/bin/msvcr100.dll
+endif

?
Comment 12 Mike Hommey [:glandium] 2011-08-25 01:52:41 PDT
http://hg.mozilla.org/mozilla-central/rev/9b6e97e91525
Comment 13 Serge Gautherie (:sgautherie) 2011-10-21 08:16:19 PDT
(In reply to Takeshi Ichimaru from comment #11)
> on toolkit/mozapps/installer/packager.mk,

Fixed in bug 681893.

Note You need to log in before you can comment on or make changes to this bug.