Last Comment Bug 648866 - Remove WinCE code from main build system + that of js/src/
: Remove WinCE code from main build system + that of js/src/
Status: VERIFIED FIXED
fixed-in-bs
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Windows CE
: -- normal (vote)
: mozilla6
Assigned To: Ed Morley [:emorley]
:
Mentors:
Depends on:
Blocks: 547497 614720 647389 745998
  Show dependency treegraph
 
Reported: 2011-04-10 08:36 PDT by Ed Morley [:emorley]
Modified: 2012-04-16 16:04 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Changes to js/src build system (based on TM rev 4ace629bb676) (29.20 KB, patch)
2011-04-10 09:50 PDT, Ed Morley [:emorley]
no flags Details | Diff | Splinter Review
Remove from both main b-s and js/src - v1 (173.07 KB, patch)
2011-04-13 10:03 PDT, Ed Morley [:emorley]
ted: review+
Details | Diff | Splinter Review
Remove from main b-s and js/src/ - v1 (minus config.sub parts) (168.32 KB, patch)
2011-04-18 15:59 PDT, Ed Morley [:emorley]
emorley: review+
Details | Diff | Splinter Review

Description Ed Morley [:emorley] 2011-04-10 08:36:53 PDT
Bug 647389 removed WINCE/WINMO code from most of spidermonkey. However, it was requested that the following parts be dealt with in a separate bug:
- js/src/configure.in
- js/src/config/*
- js/src/build/*

From bug 647389 comment 30:

> You've hit yet another nasty secret we need to document.
> 
> So the build system in js/src is shared with the main build system, so we can't
> make the changes to it independently of the build system. (You can check this
> by running |make check-sync-dirs|. So I think you should split this up a bit:
> 
> - any changes in js/src/config/, js/src/build or js/src/configure.in should be
> split into a follow-on bug. The same changes should be made to the same files
> in the root dir, and you should check with :ted to make sure you're on the
> right track (ask for an f? with the existing diff on those files in js/src).
Comment 1 Ed Morley [:emorley] 2011-04-10 09:50:33 PDT
Created attachment 524961 [details] [diff] [review]
Changes to js/src build system (based on TM rev 4ace629bb676)

The attached patch just contains the changes to the js/src build system (split from bug 647389).

Ted, can you advise as to:

1) What corresponding files in the main build system need to have the same changes made; so that |make check-sync-dirs| doesn't fail?

ie: is it just:
/configure.in
/build/autoconf/config.sub
/config/autoconf.mk.in
/config/config.mk
/config/rules.mk

2) Whether it's ok for me to deal with the following at the same time?
/allmakefiles.sh
/Makefile.in
/build/binary-location.mk
/build/Makefile.in
/build/package/wince/*
/build/wince/*

3) Which repo I should be making changes against?
- mozilla-central
- tracemonkey (for the js/src changes)
- http://hg.mozilla.org/projects/build-system/

Thanks!
Comment 2 Ted Mielczarek [:ted.mielczarek] 2011-04-11 05:46:13 PDT
I'll take a look at your patch soon, but to address your other comments:
http://mxr.mozilla.org/mozilla-central/source/js/src/Makefile.in#571

check-sync-dirs checks the config/ and build/ directories, but there's also an exceptions list per-directory, like:
http://mxr.mozilla.org/mozilla-central/source/js/src/config/check-sync-exceptions

configure.in is not on the list of synced files (the configures differ quite a bit), but our current thinking is that if you are changing something that is common to both files, you should change it in both places.

You can change whatever other files you like in your patch, as long as you do the right thing so that check-sync-dirs passes.

m-c, t-m, and b-s are all fairly regularly synced, so there shouldn't be much divergence, and a patch from one should apply to the others fairly easily. If you're going to touch a lot of build system bits, I'd prefer if the patch landed on b-s. I admit this is a gray area, since you're touching JS build bits. Thanks for asking, though! If you need someone to land your patch there, feel free to ask.
Comment 3 Ed Morley [:emorley] 2011-04-11 17:16:30 PDT
Comment on attachment 524961 [details] [diff] [review]
Changes to js/src build system (based on TM rev 4ace629bb676)

Thanks for the info regarding check-sync-dirs and preferred repos :-)

In fact, I was thinking that perhaps if I'm going to be touching a fair amount of the build system to remove WinCE code - that it might be a good time to remove other unsupported platforms at the same time - if only to avoid repeated code churn by doing them all separately, causing everyone else's patches to rot every 5 minutes. Is there a proper list somewhere of what is no longer supported?

ie: From this etherpad: http://etherpad.mozilla.com:9000/OvUWA6CtDt
>- Kill old platforms.  There are a bunch of old platforms that take a bunch 
> of rules.mk hacks.  Does anyone care about SunOS?  OpenVMS?  Old Unixes 
> need to die.

...there seems to be a desired to remove old unix platforms too. 

What do you think?

Thanks!
Comment 4 Paul Biggar 2011-04-11 17:41:16 PDT
(In reply to comment #3) 
> In fact, I was thinking that perhaps if I'm going to be touching a fair amount
> of the build system to remove WinCE code - that it might be a good time to
> remove other unsupported platforms at the same time

Danger, danger!!!! I went down this rabbit hole, and I don't recommend it.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-04-12 05:33:22 PDT
Yeah, I would stick to doing one thing at a time. Otherwise you'll be forever chasing cleanup and un-bitrotting your patches. Just get this done and landed, and you can move on to something else. There's no shortage of stuff to clean up or fix. :)
Comment 6 Ed Morley [:emorley] 2011-04-12 17:52:07 PDT
Good points - thanks for the advice (and avoidance of inevitable pain and suffering!).

I've almost finished the patch for removal of just wince code from main+js build systems, just a quick question:

Is/was OGGLES only used for WinCE? If so, I can remove a little bit more from configure.in and autoconf.mk.in:
http://mxr.mozilla.org/mozilla-central/search?string=OGLES
Comment 7 Ed Morley [:emorley] 2011-04-13 09:32:58 PDT
Clarifying title now that this bug is about removing WinCE code from all of the main b-s, rather than only the parts needed to keep check-sync-dirs happy.
Comment 8 Ed Morley [:emorley] 2011-04-13 10:03:05 PDT
Created attachment 525705 [details] [diff] [review]
Remove from both main b-s and js/src - v1

Removes all WinCE/WinMo code from root build files, build/, config/ and js/src equivalents. 

Have also removed HAS_OGLES and OGLES_SDK_DIR defines, given that the only consumers of them were the WinCE build tools (removed by this patch) and an ifdef in toolkit that was only reached |ifeq ($(OS_ARCH),WINCE)|. 

I will remove the tookit HAS_OGLES reference in a follow-up since presumably it will need a different reviewer / will want to land on m-c rather than b-s.
Comment 9 Ted Mielczarek [:ted.mielczarek] 2011-04-15 11:30:07 PDT
Comment on attachment 525705 [details] [diff] [review]
Remove from both main b-s and js/src - v1

This looks 99% okay, but please don't touch config.sub. We generally take that wholesale from upstream. (It's possible we patched it locally for this, but let's just leave that be for now.)

r=me with those bits left out.
Comment 10 Ed Morley [:emorley] 2011-04-18 15:36:57 PDT
Ah right; didn't know it was from upstream, thanks for pointing it out. 

In fact, the included version of config.sub is quite out of date now compared to http://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.sub;hb=HEAD (2009-12-04 vs latest 2011-03-23), is it worth filing a bug to update it along the same lines as bug 515002?
Comment 11 Ed Morley [:emorley] 2011-04-18 15:59:07 PDT
Created attachment 526860 [details] [diff] [review]
Remove from main b-s and js/src/ - v1 (minus config.sub parts)

Carrying forward r+ since identical to previous except minus config.sub changes and updated to tip (b-s project repo tip).
Comment 12 Mitchell Field [:Mitch] 2011-04-18 21:42:44 PDT
Thanks! Pushed with conflicts fixed:
http://hg.mozilla.org/projects/build-system/rev/0b3b74b8f087
Comment 13 Ed Morley [:emorley] 2011-04-19 02:44:09 PDT
Thanks! Yeah sorry the b-s merge that happened after I updated the patch to tip changed quite a bit, thanks for resolving the conflicts :-)
Comment 14 Mitchell Field [:Mitch] 2011-04-19 07:46:35 PDT
http://hg.mozilla.org/mozilla-central/rev/0b3b74b8f087
Comment 15 Ed Morley [:emorley] 2011-05-02 17:14:36 PDT
http://mxr.mozilla.org/mozilla-central/search?string=wince
...shows no hits (other than config.sub, which was asked to be left) in:
/
/build/
/config/
/js/src/
/js/src/build/
/js/src/config

-> Verified.

Note You need to log in before you can comment on or make changes to this bug.