Closed Bug 1257435 Opened 8 years ago Closed 8 years ago

JS JIT tests failing on VS2015 builds due to missing universal CRT DLLs

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(3 files)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=726358a7617b (and presumably https://treeherder.mozilla.org/#/jobs?repo=try&revision=67fec88c4f61) has red JIT tests. Error code says we're missing a DLL. I have no clue which. Likely a C++ redistributable runtime or CRT DLL. I should be able to reproduce in a VM.
I couldn't reproduce this in the main VM I've been using for testing :/ Although it's possible I accidentally installed the C++ redistributable runtime on that VM. I'll try with a fresh VM tomorrow.
Launching js.exe is complaining on a fresh VM because some universal CRT DLLs aren't present (api-ms-win-crt-*.dll). The jsshell zip archive contains vcruntime140.dll but not the universal CRT files. We'll need to package those DLLs in the jsshell zip like we do in the Firefox zip.
Summary: JS JIT tests failing on VS2015 builds (presumably due to missing DLL) → JS JIT tests failing on VS2015 builds (presumably due to missing universal CRT DLLs)
And ucrtbase.dll of course.
Builds with Visual Studio 2015 require the Universal CRT. While the
universal CRT may be present on the target machine, there is no
guarantee of this. So, we have to package these DLLs to ensure target
machines are able to run js.exe.

Similar code to what this commit adds exists in build/win32/Makefile.in.

Review commit: https://reviewboard.mozilla.org/r/40829/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40829/
Attachment #8731783 - Flags: review?(ted)
Summary: JS JIT tests failing on VS2015 builds (presumably due to missing universal CRT DLLs) → JS JIT tests failing on VS2015 builds due to missing universal CRT DLLs
Comment on attachment 8731782 [details]
MozReview Request: Bug 1257435 - Reformat make syntax to make easier to read; r?glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40827/diff/1-2/
Comment on attachment 8731783 [details]
MozReview Request: Bug 1257435 - Package Universal CRT DLLs with jsshell; r?glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40829/diff/1-2/
https://archive.mozilla.org/pub/firefox/try-builds/gszorc@mozilla.com-5d73b8a3fe6b54a21c545cfa1e712ca47213bc51/try-win32/jsshell-win32.zip is now packaging all the UCRT files:

Archive:  jsshell-win32.zip
  Length     Date   Time    Name
 --------    ----   ----    ----
  4656128  01-01-10 00:00   js.exe
   104448  01-01-10 00:00   mozglue.dll
    85840  01-01-10 00:00   vcruntime140.dll
   443192  01-01-10 00:00   msvcp140.dll
  1597952  01-01-10 00:00   nss3.dll
    19136  01-01-10 00:00   api-ms-win-core-processthreads-l1-1-1.dll
    20672  01-01-10 00:00   api-ms-win-core-synch-l1-1-0.dll
    19648  01-01-10 00:00   api-ms-win-crt-heap-l1-1-0.dll
    18112  01-01-10 00:00   api-ms-win-core-rtlsupport-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-file-l2-1-0.dll
    19136  01-01-10 00:00   api-ms-win-crt-environment-l1-1-0.dll
    24768  01-01-10 00:00   api-ms-win-crt-string-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-core-interlocked-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-handle-l1-1-0.dll
    23232  01-01-10 00:00   api-ms-win-crt-runtime-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-timezone-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-core-synch-l1-2-0.dll
    18624  01-01-10 00:00   api-ms-win-core-string-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-datetime-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-core-console-l1-1-0.dll
    73408  01-01-10 00:00   api-ms-win-crt-private-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-crt-utility-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-crt-locale-l1-1-0.dll
    20672  01-01-10 00:00   api-ms-win-crt-filesystem-l1-1-0.dll
    19648  01-01-10 00:00   api-ms-win-crt-conio-l1-1-0.dll
    24768  01-01-10 00:00   api-ms-win-crt-stdio-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-core-libraryloader-l1-1-0.dll
    19648  01-01-10 00:00   api-ms-win-core-sysinfo-l1-1-0.dll
    22208  01-01-10 00:00   api-ms-win-core-file-l1-1-0.dll
    26816  01-01-10 00:00   api-ms-win-crt-multibyte-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-util-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-core-heap-l1-1-0.dll
    22720  01-01-10 00:00   api-ms-win-crt-convert-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-file-l1-2-0.dll
    21184  01-01-10 00:00   api-ms-win-crt-time-l1-1-0.dll
    18112  01-01-10 00:00   api-ms-win-core-profile-l1-1-0.dll
    19136  01-01-10 00:00   api-ms-win-core-memory-l1-1-0.dll
    19648  01-01-10 00:00   api-ms-win-core-processenvironment-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-errorhandling-l1-1-0.dll
    20672  01-01-10 00:00   api-ms-win-core-processthreads-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-namedpipe-l1-1-0.dll
    29376  01-01-10 00:00   api-ms-win-crt-math-l1-1-0.dll
    21184  01-01-10 00:00   api-ms-win-core-localization-l1-2-0.dll
    19648  01-01-10 00:00   api-ms-win-crt-process-l1-1-0.dll
    18624  01-01-10 00:00   api-ms-win-core-debug-l1-1-0.dll
   921280  01-01-10 00:00   ucrtbase.dll
 10429952  01-01-10 00:00   icudt56.dll
  1318400  01-01-10 00:00   icuin56.dll
   914944  01-01-10 00:00   icuuc56.dll
 --------                   -------
 21335880                   49 files

running js.exe from a fresh Windows 7 VM without the UCRT installed globally is able to run js.exe just fine. (Before it was complaining about missing api-ms-win* DLLs.)
Attachment #8731782 - Flags: review?(ted) → review?(mh+mozilla)
Attachment #8731783 - Flags: review?(ted) → review?(mh+mozilla)
Comment on attachment 8731782 [details]
MozReview Request: Bug 1257435 - Reformat make syntax to make easier to read; r?glandium

https://reviewboard.mozilla.org/r/40827/#review37493

::: toolkit/mozapps/installer/upload-files.mk:74
(Diff revision 2)
> -endif
> +  endif
> -ifdef MSVC_CXX_RUNTIME_DLL
> +  ifdef MSVC_CXX_RUNTIME_DLL
> -JSSHELL_BINS += $(MSVC_CXX_RUNTIME_DLL)
> +    JSSHELL_BINS += $(MSVC_CXX_RUNTIME_DLL)
> -endif
> +  endif
> -ifdef MOZ_FOLD_LIBS
> +  ifdef MOZ_FOLD_LIBS
> -JSSHELL_BINS += $(DLL_PREFIX)nss3$(DLL_SUFFIX)
> +  JSSHELL_BINS += $(DLL_PREFIX)nss3$(DLL_SUFFIX)

Missing indentation here
Attachment #8731782 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8731783 [details]
MozReview Request: Bug 1257435 - Package Universal CRT DLLs with jsshell; r?glandium

https://reviewboard.mozilla.org/r/40829/#review37495

::: toolkit/mozapps/installer/upload-files.mk:67
(Diff revision 2)
>    ifdef MSVC_C_RUNTIME_DLL
>      JSSHELL_BINS += $(MSVC_C_RUNTIME_DLL)
>    endif
>    ifdef MSVC_CXX_RUNTIME_DLL
>      JSSHELL_BINS += $(MSVC_CXX_RUNTIME_DLL)
>    endif

Please insert after those lines instead... which, btw, shouldn't be in the ifndef MOZ_SYSTEM_NSPR
Attachment #8731783 - Flags: review?(mh+mozilla)
Comment on attachment 8731782 [details]
MozReview Request: Bug 1257435 - Reformat make syntax to make easier to read; r?glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40827/diff/2-3/
Attachment #8731782 - Attachment description: MozReview Request: Bug 1257435 - Reformat make syntax to make easier to read; r?ted → MozReview Request: Bug 1257435 - Reformat make syntax to make easier to read; r?glandium
Comment on attachment 8731783 [details]
MozReview Request: Bug 1257435 - Package Universal CRT DLLs with jsshell; r?glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40829/diff/2-3/
Attachment #8731783 - Attachment description: MozReview Request: Bug 1257435 - Package Universal CRT DLLs with jsshell; r?ted → MozReview Request: Bug 1257435 - Package Universal CRT DLLs with jsshell; r?glandium
Attachment #8731783 - Flags: review?(mh+mozilla)
Comment on attachment 8731952 [details]
MozReview Request: Bug 1257435 - Move MSVC runtime DLL code outside of MOZ_SYSTEM_NSPR block; r?glandium

https://reviewboard.mozilla.org/r/40959/#review37545
Attachment #8731952 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8731783 [details]
MozReview Request: Bug 1257435 - Package Universal CRT DLLs with jsshell; r?glandium

https://reviewboard.mozilla.org/r/40829/#review37547
Attachment #8731783 - Flags: review?(mh+mozilla) → review+
https://reviewboard.mozilla.org/r/40959/#review37549

You know, I noticed this same thing while touching this file in my ICU patch stack the other day but forgot to do anything about it. It looks like it was just a mistake in this patch:
https://hg.mozilla.org/mozilla-central/rev/bf88ac3f2316
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.