523964 - CROSS_COMPILE jscpucfg on OSX assumes OSX->OSX

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[ash_mozilla]

Attachment: Patch v1

This assumes that when __APPLE__ is defined in the host that the target is also __APPLE__ - this means you can't cross compile from OSX to windows using MinGW. 

The way of detecting the cross-compile form might not be right, but the change to Makefile.in to let JSCPUCFG_DEFINES be taken from cmdline let this at least work.

(Oh yeah, this patched file was from a working copy with bug #523162 applied, but this patch should still apply - just the line numbers will be out a bit)

Comment 1

[Jason Orendorff [:jorendorff]]

Comment on attachment 407872 [details] [diff] [review]
Patch v1

I don't see anything wrong with that, but jimb is a better reviewer for this patch.

Jim, if you want to just stamp it, I can land; or I can Try Server it first if you prefer.

Comment 2

[Jim Blandy :jimb]

The fact that we run jscpucfg when cross-compiling makes me want to cry.

Comment 3

[Jim Blandy :jimb]

So... When one is cross-compiling on an OSX host for a Windows target using MinGW, are both __APPLE__ and WIN32 #defined?  Are both those definitions provided by the compiler, or are they coming from Mozilla headers?

Comment 4

[ash_mozilla]

(In reply to comment #3)
> So... When one is cross-compiling on an OSX host for a Windows target using
> MinGW, are both __APPLE__ and WIN32 #defined?  Are both those definitions
> provided by the compiler, or are they coming from Mozilla headers?

jscpucfg is compiled using the native compiler (c++ in this case) – the rest of SM is build using the host compiler (i386-mingw-g++ here).

So __APPLE__ is defined since it  is compiled using the normal apple compiler, WIN32 comes in from the CROSS_COMPILE ACDEFINES. The full compile string for jscpucfg is currently:

c++ -DMDCPUCFG=\"md/_win95.cfg\"  -DCROSS_COMPILE=1 -DWINVER=0x501 -D_WIN32_WINNT=0x501 -D_WIN32_IE=0x0500 -DHAVE_SNPRINTF=1 -D_WINDOWS=1 -DWIN32=1 -DXP_WIN=1 -DXP_WIN32=1 -DHW_THREADS=1 -DSTDC_HEADERS=1 -DNEW_H=\<new\> -DWIN32_LEAN_AND_MEAN=1 -DNO_X11=1 -D_X86_=1 -DD_INO=d_ino -DFEATURE_NANOJIT=1 -DJS_TRACER=1 -DAVMPLUS_IA32=1 -DAVMPLUS_WIN32=1 -DSTDC_HEADERS=1 -DJS_HAVE_STDINT_H=1 -DJS_BYTES_PER_WORD=4 -DJS_BITS_PER_WORD_LOG2=5 -DJS_ALIGN_OF_POINTER=4 -DJS_BYTES_PER_DOUBLE=8 -DHAVE_DIRENT_H=1 -DHAVE_GETOPT_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_MALLOC_H=1 -DNEW_H=\<new\> -DHAVE_LIBM=1 -DHAVE_GETPAGESIZE=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_SNPRINTF=1 -DHAVE_STRERROR=1 -DHAVE_SYSTEMTIMETOFILETIME=1 -DHAVE_GETSYSTEMTIMEASFILETIME=1 -DHAVE_WCRTOMB=1 -DHAVE_MBRTOWC=1 -DHAVE_CPP_EXPLICIT=1 -DHAVE_CPP_TYPENAME=1 -DHAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX=1 -DHAVE_CPP_PARTIAL_SPECIALIZATION=1 -DHAVE_CPP_ACCESS_CHANGING_USING=1 -DHAVE_CPP_AMBIGUITY_RESOLVING_USING=1 -DHAVE_CPP_NAMESPACE_STD=1 -DHAVE_CPP_UNAMBIGUOUS_STD_NOTEQUAL=1 -DHAVE_CPP_NEW_CASTS=1 -DNEED_CPP_UNUSED_IMPLEMENTATIONS=1 -DHAVE___CXA_DEMANGLE=1 -DHAVE__UNWIND_BACKTRACE=1 -DCPP_THROW_NEW=throw\(\) -DMOZ_DLL_SUFFIX=\".dll\" -DHAVE_SETLOCALE=1  -DOSTYPE=\"WINNT\" -DOSARCH=WINNT -DEXPORT_JS_API   -o jscpucfg ../src/jscpucfg.cpp

Comment 5

[(not currently active) Ted Mielczarek]

jscpucfg needs to just die.

Comment 6

[Jim Blandy :jimb]

Comment on attachment 407872 [details] [diff] [review]
Patch v1

Instead of this:

#if !defined(WIN32) && defined(__APPLE__)

Could you try out this?

#if defined(XP_UNIX) && defined(__APPLE__)

Testing __APPLE__ there at all, if I understand the story, is stupid: it's making a decision about what code the target should consume by testing a host compiler's preprocessor symbol.  It's just loopy.  At least XP_UNIX accurately reflects the target we're compiling for.

The right fix, though, is not to use jscpucfg for cross-compilation.

Comment 7

[ash_mozilla]

(In reply to comment #6)
> Could you try out this?
> 
> #if defined(XP_UNIX) && defined(__APPLE__)
> 
> Testing __APPLE__ there at all, if I understand the story, is stupid: it's
> making a decision about what code the target should consume by testing a host
> compiler's preprocessor symbol.  It's just loopy.  At least XP_UNIX accurately
> reflects the target we're compiling for.

Can't test this right now, but given the XP_WIN and XP_WIN32 are defined (see comment #4) it probably would.

> The right fix, though, is not to use jscpucfg for cross-compilation.

How would this be done then? Move all the endian tests to a runtime const function/variable property of the library something like http://github.com/ruediger/flusspferd/blob/master/src/core/encodings.cpp#L59-64 perhaps?

Comment 8

[Jim Blandy :jimb]

I filed bug 467453 for this a while ago.

Comment 9

[Jim Blandy :jimb]

Comment on attachment 407872 [details] [diff] [review]
Patch v1

Needs changes requested in comment #6.

Comment 10

[(not currently active) Ted Mielczarek]

bug 694499 got rid of jscpucfg