948301 - Unable to compile js shells on Windows

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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)

Comment 3

[(no longer active)]

I'll try to see if I can reproduce.

Comment 4

[(no longer active)]

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.

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Second build failure log with bash commands

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

Comment 6

[(no longer active)]

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.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(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.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Yes, I think ICU does not like pymake, and I thought pymake is going away? (I may be wrong here)

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Does anyone on the JS team compile / develop on Windows?

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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.

Comment 11

[(no longer active)]

I managed to reproduce.

Comment 12

[(no longer active)]

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.

Comment 13

[(no longer active)]

Attachment: Patch (v1)

Comment 14

[(no longer active)]

https://tbpl.mozilla.org/?tree=Try&rev=6d32c3df4e40

Comment 15

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

This compiles properly now.

Comment 16

[(no longer active)]

The first try push was bogus, this one is real: https://tbpl.mozilla.org/?tree=Try&rev=351b2644f61c

Comment 17

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I think that after this lands on m-c, this might be needed on Aurora as well.

Comment 18

[(no longer active)]

(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 19

[Mike Hommey [:glandium]]

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.

Comment 20

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> (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.

Comment 21

[(no longer active)]

(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".

Comment 22

[Mike Hommey [:glandium]]

(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.

Comment 23

[(no longer active)]

Oh, right!

Comment 24

[(no longer active)]

Attachment: WIP

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.

Comment 25

[(no longer active)]

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.

Comment 26

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Mike, do you think you can fix this? Please?

Comment 27

[Mike Hommey [:glandium]]

Attachment: Fix static ICU build in js standalone builds

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.

Comment 28

[(not currently active) Ted Mielczarek]

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?

Comment 29

[Mike Hommey [:glandium]]

(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

Comment 30

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc128b242d8a

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 32

[Benjamin Kerensa [:bkerensa]]

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?

Comment 33

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(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.

Comment 34

[Benjamin Kerensa [:bkerensa]]

We are not going to track this but please feel free to nominate this for uplift in Aurora.

Comment 35

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Shall we nominate this for mozilla-aurora?

Comment 36

[(not currently active) Ted Mielczarek]

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.)

Comment 37

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(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. :)

Comment 38

[(not currently active) Ted Mielczarek]

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.

Comment 39

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

Comment 40

[u279076]

Setting status-firefox29 flag to verified as per comment 37.

Comment 41

[(no longer active)]

Technically this is NPOTB so it doesn't require approval.

Comment 42

[Lukas Blakk [:lsblakk] use ?needinfo]

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

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/24e822034332

Comment 44

[u279076]

Gary, can you please verify this is fixed in Aurora now that it's landed?

Comment 45

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Yes, Windows compilation errors have been fixed on 28 and 29.