Closed Bug 948301 Opened 6 years ago Closed 6 years ago

Unable to compile js shells on Windows

Categories

(Core :: JavaScript Engine, defect, blocker)

x86
Windows 7
defect
Not set
blocker

Tracking

()

VERIFIED FIXED
mozilla29
Tracking Status
firefox26 --- unaffected
firefox27 --- unaffected
firefox28 - verified
firefox29 - verified
firefox-esr17 --- unaffected
firefox-esr24 --- unaffected

People

(Reporter: gkw, Assigned: glandium)

References

(Depends on 1 open bug)

Details

(Keywords: regression, Whiteboard: [fuzzblocker])

Attachments

(3 files, 2 obsolete files)

Attached file build log
+++ This bug was initially created as a clone of Bug #947683 +++

Build failure on Windows is still not yet fixed. Tested with a threadsafe deterministic 32-bit js shell.
Also occurs with --disable-threadsafe shells.

AR=ar sh ./configure --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --disable-threadsafe

===

Ending part of the build log:

Unified_cpp_shell0.obj
Unified_cpp_shell0.cpp
c:\users\fuzz1win\desktop\js-dbg-32-dm-windows-mozilla-central-159571-75c0c92d7fa4-j812aj\compilepath\js\src\vm/RegExpObject.h(445) : warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template
c:\users\fuzz1win\desktop\js-dbg-32-dm-windows-mozilla-central-159571-75c0c92d7fa4-j812aj\compilepath\js\src\vm/ErrorObject.h(37) : warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template
c:\users\fuzz1win\desktop\js-dbg-32-dm-windows-mozilla-central-159571-75c0c92d7fa4-j812aj\compilepath\js\src\jscompartment.h(504) : warning C4805: '==' : unsafe mix of type 'js::AutoDebugModeInvalidation::<unnamed-type-needInvalidation_>' and type 'bool' in operation
c:\users\fuzz1win\desktop\js-dbg-32-dm-windows-mozilla-central-159571-75c0c92d7fa4-j812aj\compilepath\js\src\vm/StringObject.h(65) : warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template
c:\Users\fuzz1win\Desktop\js-dbg-32-dm-windows-mozilla-central-159571-75c0c92d7fa4-j812aj\compilePath\js\src\shell\js.cpp(5003) : warning C4805: '==' : unsafe mix of type 'int32_t' and type 'bool' in operation
c:\Users\fuzz1win\Desktop\js-dbg-32-dm-windows-mozilla-central-159571-75c0c92d7fa4-j812aj\compilePath\js\src\shell\js.cpp(5090) : warning C4805: '==' : unsafe mix of type 'int32_t' and type 'bool' in operation
c:\Users\fuzz1win\Desktop\js-dbg-32-dm-windows-mozilla-central-159571-75c0c92d7fa4-j812aj\compilePath\js\src\shell\js.cpp(5189) : warning C4805: '==' : unsafe mix of type 'int32_t' and type 'bool' in operation
js_static.lib
Executing: lib -NOLOGO -OUT:js_static.lib jitprofiling.obj Compression.obj Decimal.obj Parser.obj jsarray.obj jsatom.obj jsmath.obj jsutil.obj ExecutableAllocatorWin.obj OSAllocatorWin.obj ARMAssembler.obj MacroAssemblerARM.obj MacroAssemblerX86Common.obj YarrJIT.obj pm_stub.obj Unified_cpp_0.obj Unified_cpp_1.obj Unified_cpp_2.obj Unified_cpp_3.obj Unified_cpp_4.obj Unified_cpp_5.obj Unified_cpp_6.obj Unified_cpp_7.obj Unified_cpp_8.obj Unified_cpp_9.obj Unified_cpp_10.obj Unified_cpp_11.obj ./intl/icu/target/lib/icuin.lib ./intl/icu/target/lib/icuuc.lib ./intl/icu/target/lib/icudt.lib
LINK : fatal error LNK1181: cannot open input file './intl/icu/target/lib/icuin.lib'

../config/rules.mk:849: recipe for target 'js_static.lib' failed
mozmake[1]: *** [js_static.lib] Error 1181
../config/rules.mk:596: recipe for target 'default' failed
mozmake: *** [default] Error 2
Still fails as of m-c tip df82be9d89a5 which has the fix for bug 947299. Jan suggests this may be related to Ehsan's fix there... (the ball seems to be going back and forth now)
Flags: needinfo?(ehsan)
I'll try to see if I can reproduce.
Flags: needinfo?(ehsan)
I get the following error when building:

c:\Users\ehsan\moz\src\js\src\obj\intl\icu\target\data\Makefile:486:38:order-onl
y prerequisites not implemented

I think that's because I ran into bug 948534 and I had to use pymake forcefully (see bug 948534 comment 0) and it seems like the ICU makefiles do not like pymake.

Please either provide me with *exact* steps to reproduce this build failure (starting from opening the mozilla build shell) or ask somebody who can build the js shell on Windows to work on it.  Note that I'm using the latest MozillaBuild prerelease from RyanVM's people account which inclused mozmake.  Also note that I asked for help on #jsapi to build the js shell on Windows to no avail.
Depends on: 948534
Flags: needinfo?(gary)
I'm using MozillaBuild 1.9.0 prerelease, on Windows 7, on MSVC 2010, used start-shell-msvc2010.bat

I ran `autoconf-2.13`.

I used the following parameters:

AR=ar sh ./configure --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --disable-threadsafe

I ran the following command:

mozmake -C <objdir location> -j10 -s
Flags: needinfo?(gary) → needinfo?(ehsan)
Sorry I still get the error in comment 4 during configure.  Not sure if I can help here unless you get someone to fix bug 948534.
Flags: needinfo?(ehsan)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #6)
> Sorry I still get the error in comment 4 during configure.  Not sure if I
> can help here unless you get someone to fix bug 948534.

I hit that error too when I used make in 1.8.0. I made it go away when I trashed my MozillaBuild folder, installed 1.9.0, and called mozmake instead.
Yes, I think ICU does not like pymake, and I thought pymake is going away? (I may be wrong here)
Does anyone on the JS team compile / develop on Windows?
Flags: needinfo?(jorendorff)
Flags: needinfo?(jdemooij)
I'm at a loss here. Ehsan mentions there seems to be an "incorrect check in config/baseconfig.mk" (quoted on IRC), so setting needinfo? from gps.
Flags: needinfo?(gps)
I managed to reproduce.
Flags: needinfo?(jorendorff)
Flags: needinfo?(jdemooij)
Flags: needinfo?(gps)
Here are the library names that we need to link against on Windows:

sicuind.lib  sicutud.lib  sicuucd.lib

I have no idea how this worked before, but I have a patch which fixes things.
Assignee: nobody → ehsan
Attached patch Patch (v1) (obsolete) — Splinter Review
Attachment #8345537 - Flags: review?(mh+mozilla)
Comment on attachment 8345537 [details] [diff] [review]
Patch (v1)

This compiles properly now.
Attachment #8345537 - Flags: feedback+
The first try push was bogus, this one is real: https://tbpl.mozilla.org/?tree=Try&rev=351b2644f61c
I think that after this lands on m-c, this might be needed on Aurora as well.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #16)
> The first try push was bogus, this one is real:
> https://tbpl.mozilla.org/?tree=Try&rev=351b2644f61c

Sigh. https://tbpl.mozilla.org/?tree=Try&rev=114ae21a6de1

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #17)
> I think that after this lands on m-c, this might be needed on Aurora as well.

Indeed.
Comment on attachment 8345537 [details] [diff] [review]
Patch (v1)

(splinter doesn't like this patch)

The configure.in part is not useful.

What you need, instead, is to add $(DEPTH) to ICU_FILES in Makefile.in. That will make it match MOZ_ICU_LIBS, and thus make the build dependencies right.
Attachment #8345537 - Flags: review?(mh+mozilla) → review-
> (splinter doesn't like this patch)

That's probably because (as I figured out and mentioned to Ehsan over IRC) it's encoded in UTF-16 instead of ASCII.
(In reply to Mike Hommey [:glandium] from comment #19)
> Comment on attachment 8345537 [details] [diff] [review]
> Patch (v1)
> 
> (splinter doesn't like this patch)
> 
> The configure.in part is not useful.
> 
> What you need, instead, is to add $(DEPTH) to ICU_FILES in Makefile.in. That
> will make it match MOZ_ICU_LIBS, and thus make the build dependencies right.

I'm not sure if I understand your comment.  Did you see comment 12?  The issue here is not a dependency problem, we're trying to copy files with the wrong name.  The icu static builds prefix the generated .lib files with a "s".
Flags: needinfo?(mh+mozilla)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #21)
> I'm not sure if I understand your comment.  Did you see comment 12?  The
> issue here is not a dependency problem, we're trying to copy files with the
> wrong name.  The icu static builds prefix the generated .lib files with a
> "s".

It *is* a dependency problem. See ICU_LIB_RENAME.
Flags: needinfo?(mh+mozilla)
Oh, right!
Attached patch WIP (obsolete) — Splinter Review
This is what you asked me to do, but it doesn't work, which is not entirely surprising now that I look more closely at the code, since when ICU_LIB_RENAME has a value, ICU_FILES is not even defined.
Attachment #8345537 - Attachment is obsolete: true
Attachment #8346312 - Flags: feedback?(mh+mozilla)
Actually I may not be the right owner for this bug, as clearly I don't even understand glandium's suggestion, and have no idea what the correct dependencies are, how we're getting them wrong, etc.
Assignee: ehsan → nobody
Mike, do you think you can fix this? Please?
Flags: needinfo?(mh+mozilla)
Blocks: 915735
Turns out this is a different issue from what i thought. ICU_LIB_RENAME was simply not being defined in the right branch, as it is something for non-MOZ_SHARED_ICU builds.
Attachment #8348511 - Flags: review?(ted)
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Attachment #8346312 - Attachment is obsolete: true
Attachment #8346312 - Flags: feedback?(mh+mozilla)
Flags: needinfo?(mh+mozilla)
Comment on attachment 8348511 [details] [diff] [review]
Fix static ICU build in js standalone builds

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

::: js/src/Makefile.in
@@ +153,5 @@
>    INSTALL_TARGETS += ICU
>    $(ICU_FILES): buildicu
>    ICU_TARGET := $(if $(MOZ_PSEUDO_DERECURSE),compile,export)
>  endif
> +else

With all this nesting, this could really use a comment indicating what this pairs with.

Can we move some of this logic to configure?
Attachment #8348511 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #28)
> Can we move some of this logic to configure?

Eventually this will likely go to its own moz.build/Makefile.in pair
https://hg.mozilla.org/mozilla-central/rev/dc128b242d8a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Blocks: 951587
No longer blocks: 951587
Gary, could you explain the impact this will have on our users so we can determine whether this is something we are going to track?
Flags: needinfo?(gary)
(In reply to Benjamin Kerensa [:bkerensa] from comment #32)
> Gary, could you explain the impact this will have on our users so we can
> determine whether this is something we are going to track?

Without this patch, developers cannot compile js shells on Windows for mozilla-aurora. This has no end-user impact, as it does not affect TBPL builds in any way.
Flags: needinfo?(gary)
We are not going to track this but please feel free to nominate this for uplift in Aurora.
Shall we nominate this for mozilla-aurora?
Flags: needinfo?(ted)
It's very simple and has no effect on actual Firefox builds, but I also have no stake in whether it's fixed or not, so use your judgement. :) (It should be very safe to uplift, since it's just jsshell-packaging.)
Flags: needinfo?(ted)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #36)
> It's very simple and has no effect on actual Firefox builds, but I also have
> no stake in whether it's fixed or not, so use your judgement. :) (It should
> be very safe to uplift, since it's just jsshell-packaging.)

I can verify that it's fixed.

What's needed is for the approval‑mozilla‑aurora flag to be nominated. :)
Status: RESOLVED → VERIFIED
Flags: needinfo?(ted)
Feel free. I know it's my patch, but you have more stake in this than I do. You can describe to drivers why this patch is needed.
Flags: needinfo?(ted)
Comment on attachment 8348511 [details] [diff] [review]
Fix static ICU build in js standalone builds

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Not exactly sure, this came in the line after a series of compilation errors (caused by) bug 947683 or bug 947299 or bug 915735.
User impact if declined: Unable to compile js shells for fuzzing
Testing completed (on m-c, etc.): on m-c since mid-Dec
Risk to taking this patch (and alternatives if risky): Very low risk, it's just jsshell packaging so it's something like NPOTB
String or IDL/UUID changes made by this patch: N/A
Attachment #8348511 - Flags: approval-mozilla-aurora?
Setting status-firefox29 flag to verified as per comment 37.
Technically this is NPOTB so it doesn't require approval.
Comment on attachment 8348511 [details] [diff] [review]
Fix static ICU build in js standalone builds

In that case feel free to land with a=npotb
Attachment #8348511 - Flags: approval-mozilla-aurora?
Whiteboard: [fuzzblocker] → [fuzzblocker][checkin-needed-aurora]
https://hg.mozilla.org/releases/mozilla-aurora/rev/24e822034332
Whiteboard: [fuzzblocker][checkin-needed-aurora] → [fuzzblocker]
Gary, can you please verify this is fixed in Aurora now that it's landed?
Flags: needinfo?(gary)
Yes, Windows compilation errors have been fixed on 28 and 29.
Flags: needinfo?(gary)
You need to log in before you can comment on or make changes to this bug.