420928 - default entry on win32+wince should be mainWCRTStartup

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Doug Turner (:dougt)]

See usages:

http://lxr.mozilla.org/mozilla/search?string=wmainCRTStartup

Comment 1

[Benjamin Smedberg]

Specifically: mainWCRTStartup is an alias for wmainCRTStartup on desktop compilers, but wmainCRTStartup doesn't exist on WINCE, so we should just use the alias which works on both platforms.

preemptive r=me, this should be very low-risk to take for 1.9

Comment 2

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 3

[Benjamin Smedberg]

Comment on attachment 307975 [details] [diff] [review]
patch v.1

This is low-risk as long as the tinderboxes stay green.

Comment 4

[Damon Sicore (:damons)]

Comment on attachment 307975 [details] [diff] [review]
patch v.1

a1.9+=damons

Benjamin, at some point it makes sense to stop taking patches like this so close to the release.  At least, seems that way to me.

Comment 5

[Benjamin Smedberg]

Damon, I was told that we were going to try and get 1.9 working on WINCE, so I've been reviewing that set of patches. If we would possibly take this for 1.9.0.1, we should take it now.

Comment 6

[Doug Turner (:dougt)]

Checking in browser/app/Makefile.in;
/cvsroot/mozilla/browser/app/Makefile.in,v  <--  Makefile.in
new revision: 1.150; previous revision: 1.149
done
Checking in calendar/sunbird/app/Makefile.in;
/cvsroot/mozilla/calendar/sunbird/app/Makefile.in,v  <--  Makefile.in
new revision: 1.57; previous revision: 1.56
done
Checking in mail/app/Makefile.in;
/cvsroot/mozilla/mail/app/Makefile.in,v  <--  Makefile.in
new revision: 1.83; previous revision: 1.82
done
Checking in suite/app/Makefile.in;
/cvsroot/mozilla/suite/app/Makefile.in,v  <--  Makefile.in
new revision: 1.29; previous revision: 1.28
done
Checking in toolkit/mozapps/update/src/updater/Makefile.in;
/cvsroot/mozilla/toolkit/mozapps/update/src/updater/Makefile.in,v  <--  Makefile.in
new revision: 1.26; previous revision: 1.25
done
Checking in xulrunner/app/Makefile.in;
/cvsroot/mozilla/xulrunner/app/Makefile.in,v  <--  Makefile.in
new revision: 1.37; previous revision: 1.36
done
Checking in xulrunner/stub/Makefile.in;
/cvsroot/mozilla/xulrunner/stub/Makefile.in,v  <--  Makefile.in
new revision: 1.11; previous revision: 1.10
done

Comment 7

[Mike Shaver (:shaver emeritus)]

Did this build on the try-server?  If we have a config difference between the try servers and the tinderboxes, we should resolve that.

If not, and there's a comment about "we should be OK if the tinderboxes stay green", I have to ask why the try server wasn't used.  Burning the tree even for a few cycles is very disruptive, and we're in the end game with 90 minute Windows builds.

Comment 8

[Doug Turner (:dougt)]

I agree with your suggestion about using the try server.  However, in this case, with the change being windows only, i figured I didn't need it.  Probably wouldn't hurt since they are Makefiles, and I could have screwed up a linefeed or something.  In any case, I did build the change locally and things seemed okay.  However, I was testing optimized not debug!  Ugh.  

The core problem is that it looks like in debug, we change the SUBSYSTEM linker option to be "console".  You can see this here:

http://lxr.mozilla.org/mozilla/source/browser/app/Makefile.in#147

This means that my changing the entry point will not work on windows debug.  Of course this should be documented somewhere, but it isn't:

http://msdn2.microsoft.com/en-us/library/aa908655.aspx

I would rather not have the entry point for debug be one thing, and entry point for opt be another.  The lowest risk thing for me to do for windows mobile (and the reason for this change) is to simple just key off of WINCE and change the entry point that way.  I could also keep this only in xulrunner ( we are not building browser, tb, cal on winmo)
 

Comment 9

[Doug Turner (:dougt)]

not actively working on this, feel free to help yourself.

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 307975 [details] [diff] [review]
patch v.1

Doesn't sound like doug is really worried about this, we'll let it slide.

Comment 11

[Doug Turner (:dougt)]

so... we need this to build on windows ce.  we should make these entry point changes wince only.  that will keep everything happy.

Comment 12

[Doug Turner (:dougt)]

Attachment: patch v.2 -- wince only.

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 340661 [details] [diff] [review]
patch v.2 -- wince only.

You're not actually building browser/app on WINCE, right? If not, you should probably just leave that part out.

Comment 14

[Benjamin Smedberg]

Comment on attachment 340661 [details] [diff] [review]
patch v.2 -- wince only.

What ted said, skip browser/app since you're not using that on WINCE.

Comment 15

[Doug Turner (:dougt)]

a6b61745090e for the windows ce changes.

not sure if we care about win32 or not.  leaving open, but assigning to default.

Comment 16

[Benjamin Smedberg]

Nah, let's let sleeping dogs lie.