291229 - Build config changes for Windows CE

Status: RESOLVED FIXED

(Minimo Graveyard :: Build Config, defect)

Description

[Doug Turner (:dougt)]

Comment 1

[Doug Turner (:dougt)]

Attachment: Patch v.1

This makes WindowsCE build much easier.  No more hand edited autoconf.mk files.

Comment 2

[Benjamin Smedberg]

Comment on attachment 181365 [details] [diff] [review]
Patch v.1

echo llllllllllllllllllllllllllllllllllllllllllllllllllllllllllll ${AR}

Maybe remove debugging code?

-	 DSO_LDOPTS=/SUBSYSTEM:WINDOWS
+	 dnl DSO_LDOPTS=/SUBSYSTEM:WINDOWS

What's this?

+ifeq (,$(filter-out WINCE,$(OS_ARCH)))
+XPIDL_COMPILE	= $(CYGWIN_WRAPPER) $(topsrcdir)/../tools/xpidl.exe
+XPIDL_LINK	    = $(CYGWIN_WRAPPER) $(topsrcdir)/../tools/xpt_link.exe
+endif

What?

Watch your whitespace in rules.mk, you added some extraneous end-of-line spaces
and such.

I am not qualified to review the NSS portions of this patch... they have to go
through wtc or whoever maintains NSS now.

Comment 3

[Doug Turner (:dougt)]

The xpidl tools are not built on windows ce.  no native tools are built; they
must be present before you start a windows ce build.

New patch coming.

Comment 4

[Doug Turner (:dougt)]

Attachment: Patch v.2

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #3)
> The xpidl tools are not built on windows ce.  no native tools are built; they
> must be present before you start a windows ce build.

Shouldn't that be something handled by the generic CROSS_COMPILE mechanism,
i.e., we could build the tools with HOST_CC?

Comment 6

[Doug Turner (:dougt)]

it might be able to be fixed.  HOST_* was a mess for windows -> windows cross
compilation.  Our current cross compile story assumes that we are hosting on a
unix style machine and building to something else.  I found it quite hard to
make both HOST_CC and cross compilation work.  At some point, I gave up and
enforced that you must have these two tools present when building.  it isn't too
bad since to build windows ce, you must already have tool wrappers -- you must
build a bunch of little applications that wrap cl, lib, asm.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 181454 [details] [diff] [review]
Patch v.2

>Index: configure.in
>-    dnl Hardcode to win95 for now - cls
>+dnl Hardcode to win95 for now - cls
>     TARGET_NSPR_MDCPUCFG='\"md/_win95.cfg\"'
>+

This indentation change seems random.

>Index: config/rules.mk
>@@ -549,6 +549,13 @@
> OUTOPTION = -o # eol
> endif # WINNT && !GNU_CC
> endif # VACPP
>+ifneq (,$(filter wince,$(OS_ARCH)))

Maybe use capitals here? (The inconsistency seems odd.	Does it matter?)

>+OUTOPTION = -Fo# eol
>+endif
>+
>+ifeq ($(OS_TARGET), WINCE)
>+OUTOPTION = -Fo# eol
>+endif

sr+a=dbaron

Comment 8

[Doug Turner (:dougt)]

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1443; previous revision: 1.1442
done
Checking in build/Makefile.in;
/cvsroot/mozilla/build/Makefile.in,v  <--  Makefile.in
new revision: 1.18; previous revision: 1.17
done
Checking in config/Makefile.in;
/cvsroot/mozilla/config/Makefile.in,v  <--  Makefile.in
new revision: 3.110; previous revision: 3.109
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 3.342; previous revision: 3.341
done
Checking in config/config.mk;
/cvsroot/mozilla/config/config.mk,v  <--  config.mk
new revision: 3.313; previous revision: 3.312
done
Checking in config/rules.mk;
/cvsroot/mozilla/config/rules.mk,v  <--  rules.mk
new revision: 3.469; previous revision: 3.468
done
Checking in config/static-config.mk;
/cvsroot/mozilla/config/static-config.mk,v  <--  static-config.mk
new revision: 3.20; previous revision: 3.19
done

Comment 9

[Doug Turner (:dougt)]

Attachment: Other changes to make WinCE compile.

Comment 10

[Mike Shaver (:shaver emeritus)]

Comment on attachment 181559 [details] [diff] [review]
Other changes to make WinCE compile.

Can you put up a unified diff?	It's basically impossible to see what you're
wrapping with the WINCE ifdefs without it. =/

Comment 11

[Doug Turner (:dougt)]

Attachment: u10 of patch above

Comment 12

[timeless]

Comment on attachment 181777 [details] [diff] [review]
u10 of patch above

>Index: db/mork/src/morkFile.cpp
>+#ifndef WINCE
http://msdn.microsoft.com/library/en-us/wcesdkr/html/_wcesdk_Win32_GetVersionEx
.asp?frame=true
The value of the dwPlatformID member of the OSVERSIONINFO structure will be
VER_PLATFORM_WIN32_CE.
Requirements
Runs on 	Versions	Defined in	Include 	Link to
Windows CE OS	1.0 and later	Winbase.h		Coremain.lib
>   OSVERSIONINFOA vi = { sizeof(OSVERSIONINFOA) };
>   if ((GetVersionExA(&vi) && vi.dwPlatformId == VER_PLATFORM_WIN32_WINDOWS))
>   {
>     // Win9x/ME
>     int fd = fileno(file);
>     HANDLE fh = (HANDLE)_get_osfhandle(fd);
http://msdn.microsoft.com/library/en-us/wcesdkr/html/_wcesdk_win32_FlushFileBuf
fers.asp?frame=true
Runs on 	Versions	Defined in	Include 	Link to
Windows CE OS	1.0 and later	Winbase.h		Coredll.lib, Fsmain.lib
>     FlushFileBuffers(fh);
>   }
>+#endif

>Index: dbm/src/Makefile.in
>+ifeq ($(OS_ARCH),WINCE)
>+DEFINES     += -D__STDC__ -DDBM_REOPEN_ON_FLUSH

>Index: dom/src/base/nsDOMClassInfo.cpp
>@@ -132,21 +132,23 @@
> // Oh, did I mention that I hate Microsoft for doing this to me?
>+#ifndef WINCE
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/wcesdkr/html/w
ce30oriWindowsCE30APIReference.asp
Runs on 	Versions	Defined in	Include 	Link to
Windows CE OS	1.0 and later	Winuser.h		 

Can we switch to #ifdef GetClassName ?
> #undef GetClassName
>+#endif

>Index: embedding/components/build/nsEmbeddingModule.cpp
I really wish we did:
#ifdef WINCE
#define NO_PRINTING_PROMPT_SERVICE
#endif
...

  #ifndef NO_PRINTING_PROMPT_SERVICE
instead of:
>+#ifndef WINCE

and I wish we did it for each item. I hate the individual mix and match stuff,
we have some in our tree....
> NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsPrintingPromptService, Init)
>+#endif

>Index: js/src/jsapi.c
shouldn't this go in some js specific bug so other people who hit it can find
it instead of burrying it here? :)
> #if JS_STACK_GROWTH_DIRECTION > 0
>     JS_ASSERT(dummy1addr < &dummy2);
>-    JS_ASSERT((jsuword)&dummy2 < limitAddr);
> #else
>     /* Stack grows downward, the common case on modern architectures. */
>     JS_ASSERT(&dummy2 < dummy1addr);
>-    JS_ASSERT(limitAddr < (jsuword)&dummy2);
> #endif

>Index: js/src/jscpucfg.h
too many spaces after OS2):
>+#if defined(_WIN32) || defined(XP_OS2)  || defined(WINCE)

>Index: js/src/xpconnect/shell/xpcshell.cpp
>-    if (!isatty(fileno(file))) {
http://msdn.microsoft.com/library/en-us/wceappdev5/html/wce50lrfusfileno.asp?fr
ame=true
Requirements
OS Versions: Windows CE 2.0 and later. Header: stdlib.h. Link Library:
coredll.dll.
int _fileno( 
   FILE* stream 
);
i don't understand this change, but please change js/src/js.c too if you must
make it.
>+    if (!isatty((int)fileno(file))) {

>Index: layout/base/nsCaret.cpp
>@@ -163,23 +163,24 @@
i like this change. crummy code. bad bad.
>   mBidiKeyboard = do_GetService("@mozilla.org/widget/bidikeyboard;1");
>+  if (mBidiKeyboard)

>Index: layout/generic/nsSimplePageSequence.cpp
>@@ -454,35 +454,35 @@
someone should file a bug about this:
>       PRUnichar * uStr = ToNewUnicode(formattedDateString);
>       SetDateTimeStr(uStr); // memory will be freed

>Index: modules/plugin/base/src/Makefile.in
>Index: parser/htmlparser/public/nsIHTMLContentSink.h

>-#ifdef XP_MAC
>+#if defined(XP_MAC) || defined(WINCE) 
the comment should be changed :)
> #define MAX_REFLOW_DEPTH  75    //setting to 75 to prevent layout from crashing on mac. Bug 55095.

>Index: widget/src/windows/nsWindow.cpp
>@@ -6364,20 +6368,23 @@
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/wcemain4/html/
cmconjapaneseime30.asp
Microsoft® Windows® CE .NET supports the Microsoft Input Method Editor (IME)
3.0 for Japanese. IME 3.0 allows users to type complex characters and symbols,
such as Japanese Kanji characters, by using a standard keyboard.

IME 3.0 does not include software input panels for Japanese. However, you can
use IME 3.0 with any Japanese input methods (IM) that are based on the Windows
CE software input panel architecture and provided in Windows CE .NET.

> nsWindow::HandleStartComposition(HIMC hIMEContext)
> {
>+#ifdef WINCE
>+  return false;
>+#endif

Comment 13

[Doug Turner (:dougt)]

thanks for your comments.

FlushFileBuffers works, but _get_osfhandle isn't supported on WinCE PPC2003.

We should have a MOZ_NO_PRINTING_SUPPORT, but this patch doesn't address that --
it just drops printing for WindowsCE -- something that I could support if I had
time. 

The javascript assertion change is declared in bug 242518.  I was hoping to get
shaver or brendan eyes on it.

Windows CE.net isn't the same as the PPC2003 SDK... So, some of your links into
MSDN aren't related.

I have fixed up the things you pointed out and will make a new patch.  Thanks!

Comment 14

[Doug Turner (:dougt)]

Attachment: u10 patch including timeless comments

Comment 15

[Mike Shaver (:shaver emeritus)]

Comment on attachment 182022 [details] [diff] [review]
u10 patch including timeless comments

>+#ifndef WINCE
>  typedef          long off_t;
>+#else
>+ typedef          long ino_t;
>+#endif

Can you reverse the sense of that, so that we have a positive #ifdef test
instead
of the negative #ifndef?

>+#ifndef WINCE
> #include "nsPrintingPromptService.h"
>+#endif

So there are a lot of these things that look like they want to mean "if we
don't
want printing UI", but instead say "if we're on this one platform".  Is this
the
right way to condition these things?  Would we want to turn these off if we
were
on some other constrained platform, or is the disabling due to a relatively
WinCE-specific limitation?

> #if JS_STACK_GROWTH_DIRECTION > 0
>     JS_ASSERT(dummy1addr < &dummy2);
>-    JS_ASSERT((jsuword)&dummy2 < limitAddr);
> #else
>     /* Stack grows downward, the common case on modern architectures. */
>     JS_ASSERT(&dummy2 < dummy1addr);
>-    JS_ASSERT(limitAddr < (jsuword)&dummy2);
> #endif

Why are these assertions being removed?  (Can WinCE go different directions
depending on the hardware?)

>-#if defined(XP_WIN) || defined(XP_OS2)
>+#if defined(XP_WIN) || defined(XP_OS2) || defined (WINCE)

I'm a little surprised that WINCE doesn't imply XP_WIN, but it sure doesn't
seem to from this patch.  Do we want to make this be XP_WINCE to match the
other platform defines, if it is indeed not just a way to condition some
XP_WIN stuff?

>Index: js/src/xpconnect/shell/xpcshell.cpp
>===================================================================
\>@@ -558,21 +558,21 @@
> static void
> ProcessFile(JSContext *cx, JSObject *obj, const char *filename, FILE *file)
> {
>     JSScript *script;
>     jsval result;
>     int lineno, startline;
>     JSBool ok, hitEOF;
>     char *bufp, buffer[4096];
>     JSString *str;
> 
>-    if (!isatty(fileno(file))) {
>+    if (!isatty((int)fileno(file))) {

So why is this necessary?  Does fileno really not return the type that
isatty expects?  That would be pretty loserly!

(BTW: if you add "-p" to your diff flags, you get helpful function
context.  And I know you like to be helpful!)

> #ifdef IBMBIDI
>-  PRBool isRTL;
>+  PRBool isRTL = PR_FALSE;
>   mBidiKeyboard = do_GetService("@mozilla.org/widget/bidikeyboard;1");
>-  mBidiKeyboard->IsLangRTL(&isRTL);
>+  if (mBidiKeyboard)
>+	mBidiKeyboard->IsLangRTL(&isRTL);
>   mKeyboardRTL = isRTL;
> #endif

Good robustness fix regardless.  The xplat lifestyle wins again!

>     // Create current Date/Time String
>     if (!mDateFormatter)
>       mDateFormatter = do_CreateInstance(kDateTimeFormatCID);
>-
>+#ifndef WINCE
>     NS_ENSURE_TRUE(mDateFormatter, NS_ERROR_FAILURE);
> 
>     nsAutoString formattedDateString;
>     time_t ltime;
>     time( &ltime );
>     if (NS_SUCCEEDED(mDateFormatter->FormatTime(nsnull /* nsILocale* locale */,
>                                                 kDateFormatShort,
>                                                 kTimeFormatNoSeconds,
>                                                 ltime,
>                                                 formattedDateString))) {
>       PRUnichar * uStr = ToNewUnicode(formattedDateString);
>       SetDateTimeStr(uStr); // memory will be freed
>     }
>-
>+#endif

If you don't expect to get a date formatter back, why not also skip the
do_CreateInstance?  Also, a comment explaining why this block isn't right
for WinCE would be nice.  Might even keep someone else from breaking you!

>-#ifdef XP_MAC
>+#if defined(XP_MAC) || defined(WINCE) 
> #define MAX_REFLOW_DEPTH  75    //setting to 75 to prevent layout from crashing on mac. Bug 55095.
>+                                //We will also change this for WinCE as it usually has a strict
>+                                //memory upper limit (no vm, ~32mb)

Nice comment, thanks.

> BOOL
> nsWindow::HandleStartComposition(HIMC hIMEContext)
> {
>+#ifdef WINCE
>+  return false;
>+#endif
>   // ATOK send the messages following order at starting composition.
>   // 1. WM_IME_COMPOSITION
>   // 2. WM_IME_STARTCOMPOSITION
>   // We call this function at both step #1 and #2.
>   // However, the composition start event should occur only once.
>   if (sIMEIsComposing)
>     return PR_TRUE;

If returning |false| is correct -- do all our windows compilers know
about that token? -- then returning PR_TRUE probably isn't.  Could you
fix up that return as well, while you're in there?


>+#ifndef AARONTEMP
>+		if(i==3)
>+		{
>+			// PrepareAndDispatch makes an assumption that is causing us problems.  It
[...]
>+			ap += 5;
>+		}
>+#endif /* AARONTEMP */

Do you mean to check that TEMP code in?  If so, could you give it a more
meaningful name?

I'll take your word on the rest of the xpconnect changes; they'll bust you
long before me. =)

r=shaver, please fix up as many of these nits as you think appropriate before
landing.  Also approving port-fix work for 1.8b2.

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 182022 [details] [diff] [review]
u10 patch including timeless comments

>+#ifdef WINCE
>+#ifdef ERROR
>+#undef ERROR
>+#endif
>+#endif

I'd prefer "#if defined(WINCE) && defined(ERROR)" (a couple of places),
there's no need to nest these.

>Index: js/src/xpconnect/shell/xpcshell.cpp
>===================================================================
>-    if (!isatty(fileno(file))) {
>+    if (!isatty((int)fileno(file))) {

This worries me, why doesn't fileno() already return an int? Is there
some unix port that will croak because it uses larger sized filed descriptors?

>Index: xpcom/reflect/xptcall/src/md/win32/xptcstubsce.cpp
>===================================================================

Could you do a diff -w for this one, too hard to follow what changed.

Comment 17

[Doug Turner (:dougt)]

Regarding the printing stuff -- it really isn't WinCE-specific limitation, it is
more of a time and expertise limitation.  I would expect that at some point some
bright person is going to solve the font and printing issues that windows ce
has.  I can certianlly make a MOZ_NO_PRINTTING api, but I do not think anyone
will use it as the gain is little.

As for the removal of the JS assertions -- I spoke with brendan about it... he
suggested that these can be removed without any problem.  I think it was
discussed in bug 242518 and I can site this bug when I check in.

I think I can remove this line:
+#if defined(XP_WIN) || defined(XP_OS2) || defined (WINCE)

XP_WIN is defined when WINCE is defined.  Always.  In this case, I want to
#include float.  Good catch. 

+    if (!isatty((int)fileno(file))) {

Yeah, you would expect it to work, but the PPC2003 SDK is broken.  Maybe we
shouldn't change our code but do something in the shunt layer to fix it?

As far as the xptcstubsce.cpp file -- it is new and probably doesn't need to be
throughly review this go around.... it needs to be tested further.  Aparently
the xptcall tests don't do testing of the SharedStub.  :-(

I have addressed your other concerns.  I will post another diff for reference.

Thanks shaver!!

Comment 18

[Doug Turner (:dougt)]

Dan,

The transformiix patch has changed.  We are going to drop the ERROR enum since
it isn't used.

As far as fileno, I am going to not make that change and figure out how to work
around this change.

Assuming that is okay, is that a thumbs up dan?

Comment 19

[Daniel Veditz [:dveditz]]

Everything else was OK r=me, but I'd be happier if the funky fileno() cast were
#ifdef the platform you need it on.

Comment 20

[Doug Turner (:dougt)]

Attachment: patch v.3

This is the revised patch.  sounds like I got a sr=shaver+dveditz.  objections?

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 182142 [details] [diff] [review]
patch v.3

a=asa

Comment 22

[Doug Turner (:dougt)]

Checking in db/mork/src/morkFile.cpp;
/cvsroot/mozilla/db/mork/src/morkFile.cpp,v  <--  morkFile.cpp
new revision: 1.23; previous revision: 1.22
done
Checking in dbm/include/mcom_db.h;
/cvsroot/mozilla/dbm/include/mcom_db.h,v  <--  mcom_db.h
new revision: 3.41; previous revision: 3.40
done
Checking in dbm/include/winfile.h;
/cvsroot/mozilla/dbm/include/winfile.h,v  <--  winfile.h
new revision: 3.3; previous revision: 3.2
done
Checking in dbm/src/Makefile.in;
/cvsroot/mozilla/dbm/src/Makefile.in,v  <--  Makefile.in
new revision: 3.20; previous revision: 3.19
done
Checking in dom/src/base/nsDOMClassInfo.cpp;
/cvsroot/mozilla/dom/src/base/nsDOMClassInfo.cpp,v  <--  nsDOMClassInfo.cpp
new revision: 1.277; previous revision: 1.276
done
Checking in embedding/components/Makefile.in;
/cvsroot/mozilla/embedding/components/Makefile.in,v  <--  Makefile.in
new revision: 1.17; previous revision: 1.16
done
Checking in embedding/components/build/Makefile.in;
/cvsroot/mozilla/embedding/components/build/Makefile.in,v  <--  Makefile.in
new revision: 1.51; previous revision: 1.50
done
Checking in embedding/components/build/nsEmbeddingModule.cpp;
/cvsroot/mozilla/embedding/components/build/nsEmbeddingModule.cpp,v  <-- 
nsEmbeddingModule.cpp
new revision: 1.32; previous revision: 1.31
done
Checking in js/src/Makefile.in;
/cvsroot/mozilla/js/src/Makefile.in,v  <--  Makefile.in
new revision: 3.94; previous revision: 3.93
done
Checking in js/src/jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.194; previous revision: 3.193
done
Checking in js/src/jscpucfg.h;
/cvsroot/mozilla/js/src/jscpucfg.h,v  <--  jscpucfg.h
new revision: 3.17; previous revision: 3.16
done
Checking in js/src/jslibmath.h;
/cvsroot/mozilla/js/src/jslibmath.h,v  <--  jslibmath.h
new revision: 3.17; previous revision: 3.16
done
Checking in js/src/jsnum.c;
/cvsroot/mozilla/js/src/jsnum.c,v  <--  jsnum.c
new revision: 3.66; previous revision: 3.65
done
Checking in js/src/jstypes.h;
/cvsroot/mozilla/js/src/jstypes.h,v  <--  jstypes.h
new revision: 3.32; previous revision: 3.31
done
Checking in layout/base/nsCaret.cpp;
/cvsroot/mozilla/layout/base/nsCaret.cpp,v  <--  nsCaret.cpp
new revision: 1.143; previous revision: 1.142
done
Checking in layout/generic/nsSimplePageSequence.cpp;
/cvsroot/mozilla/layout/generic/nsSimplePageSequence.cpp,v  <-- 
nsSimplePageSequence.cpp
new revision: 3.124; previous revision: 3.123
done
Checking in modules/plugin/base/src/Makefile.in;
/cvsroot/mozilla/modules/plugin/base/src/Makefile.in,v  <--  Makefile.in
new revision: 1.102; previous revision: 1.101
done
Checking in parser/htmlparser/public/nsIHTMLContentSink.h;
/cvsroot/mozilla/parser/htmlparser/public/nsIHTMLContentSink.h,v  <-- 
nsIHTMLContentSink.h
new revision: 1.16; previous revision: 1.15
done
Checking in view/src/nsViewManager.cpp;
/cvsroot/mozilla/view/src/nsViewManager.cpp,v  <--  nsViewManager.cpp
new revision: 3.401; previous revision: 3.400
done
Checking in widget/src/windows/nsWindow.cpp;
/cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 3.551; previous revision: 3.550
done
Checking in extensions/transformiix/source/xpath/ExprLexer.h;
/cvsroot/mozilla/extensions/transformiix/source/xpath/ExprLexer.h,v  <-- 
ExprLexer.h
new revision: 1.20; previous revision: 1.19
done

Comment 23

[Mike Shaver (:shaver emeritus)]

Marking FIXED for the trunk checkin.

Comment 24

[Boris Zbarsky [:bzbarsky]]

This patch disabled plugins completely if WINNT is defined at build time.  Was
that actually meant to happen?  'cause it makes plugins not work on Windows --
see bug 292373.

Comment 25

[Doug Turner (:dougt)]

ouch.  that wasn't the intent.


In modules/plugin/base/src/Makefile.in, 

ifneq (,$(filter WINNT WINCE,$(MOZ_WIDGET_TOOLKIT)))

MOZ_WIDGET_TOOLKIT should be OS_ARCH, right?

Comment 26

[Mike Shaver (:shaver emeritus)]

dougt: does that fix it?  If so, please check it in with all haste.  If it
doesn't, please back out with all haste so that we can restore the trunk.

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 181454 [details] [diff] [review]
Patch v.2

>Index: config/Makefile.in
> ifneq (,$(CROSS_COMPILE)$(filter-out WINNT OS2,$(OS_ARCH)))
>+ifneq ($(OS_ARCH), WINCE)

This should have just added WINCE to the list on the previous line.

Comment 28

[Doug Turner (:dougt)]

dbaron, i have another set of changes to the build system to make minimo use the
same toolkit as ff (bug 295482).  I will roll this change into that bug.