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).
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!
Assignee: general → bmo
Status: NEW → ASSIGNED
Assignee: bmo → nobody
QA Contact: general → build-config
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 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!
(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.
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. :)
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
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.
Summary: Remove WINCE code from js/src build system (as well as the matching places in the main build system) → Remove WinCE code from main build system + that of js/src/
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 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.
Attachment #525705 - Flags: review?(ted.mielczarek) → review+
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?
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).
Whiteboard: [needs landing on build-system repo]
Thanks! Pushed with conflicts fixed: http://hg.mozilla.org/projects/build-system/rev/0b3b74b8f087
Whiteboard: [needs landing on build-system repo] → fixed-in-bs
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 :-)
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.