Note: There are a few cases of duplicates in user autocompletion which are being worked on.

msvc*.dll missing when building with new style jemalloc

RESOLVED FIXED in mozilla9

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla9
All
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-bs)

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

6 years ago
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
(Assignee)

Comment 1

6 years ago
Created attachment 552340 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc
Attachment #552340 - Flags: review?(khuey)
(Assignee)

Comment 2

6 years ago
Comment on attachment 552340 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc

Need some more
Attachment #552340 - Flags: review?(khuey)
(Assignee)

Updated

6 years ago
Summary: msvc*.dll missing when building with VC10 → msvc*.dll missing when building with new style jemalloc
(Assignee)

Updated

6 years ago
Blocks: 678195
(Assignee)

Comment 3

6 years ago
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.
Attachment #552377 - Flags: review?(khuey)
(Assignee)

Updated

6 years ago
Attachment #552340 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Assignee: nobody → mh+mozilla
(Assignee)

Comment 4

6 years ago
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.
Attachment #552377 - Flags: review?(khuey)
(Assignee)

Comment 5

6 years ago
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.
Attachment #554318 - Flags: review?(khuey)
(Assignee)

Updated

6 years ago
Attachment #552377 - Attachment is obsolete: true
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 :-(
Attachment #554318 - Flags: review?(khuey) → review+
(Assignee)

Comment 7

6 years ago
(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.
(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.
(Assignee)

Comment 9

6 years ago
Created attachment 555090 [details] [diff] [review]
Also install msvc*.dll when using new style jemalloc
[Checked in: Comment 12]

As landing
(Assignee)

Updated

6 years ago
Attachment #554318 - Attachment is obsolete: true
(Assignee)

Comment 10

6 years ago
http://hg.mozilla.org/projects/build-system/rev/9b6e97e91525
Whiteboard: fixed-in-bs
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

?
(Assignee)

Comment 12

6 years ago
http://hg.mozilla.org/mozilla-central/rev/9b6e97e91525
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
(Assignee)

Updated

6 years ago
Depends on: 681893
Blocks: 694371
Attachment #555090 - Attachment description: Also install msvc*.dll when using new style jemalloc. → Also install msvc*.dll when using new style jemalloc [Checked in: Comment 12]
(In reply to Takeshi Ichimaru from comment #11)
> on toolkit/mozapps/installer/packager.mk,

Fixed in bug 681893.
Flags: in-testsuite-
Blocks: 652263
You need to log in before you can comment on or make changes to this bug.