Closed
Bug 648866
Opened 14 years ago
Closed 14 years ago
Remove WinCE code from main build system + that of js/src/
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla6
People
(Reporter: emorley, Assigned: emorley)
References
Details
(Whiteboard: fixed-in-bs)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
emorley
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•14 years ago
|
||
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 | ||
Updated•14 years ago
|
Assignee: bmo → nobody
Component: JavaScript Engine → Build Config
QA Contact: general → build-config
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → bmo
Comment 2•14 years ago
|
||
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.
Assignee | ||
Comment 3•14 years ago
|
||
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!
Attachment #524961 -
Flags: feedback?(ted.mielczarek)
Comment 4•14 years ago
|
||
(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•14 years ago
|
||
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. :)
Assignee | ||
Comment 6•14 years ago
|
||
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
Assignee | ||
Comment 7•14 years ago
|
||
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/
Assignee | ||
Comment 8•14 years ago
|
||
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.
Attachment #524961 -
Attachment is obsolete: true
Attachment #525705 -
Flags: review?(ted.mielczarek)
Comment 9•14 years ago
|
||
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+
Assignee | ||
Comment 10•14 years ago
|
||
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?
Assignee | ||
Comment 11•14 years ago
|
||
Carrying forward r+ since identical to previous except minus config.sub changes and updated to tip (b-s project repo tip).
Attachment #525705 -
Attachment is obsolete: true
Attachment #526860 -
Flags: review+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Whiteboard: [needs landing on build-system repo]
Comment 12•14 years ago
|
||
Thanks! Pushed with conflicts fixed:
http://hg.mozilla.org/projects/build-system/rev/0b3b74b8f087
Keywords: checkin-needed
Whiteboard: [needs landing on build-system repo] → fixed-in-bs
Assignee | ||
Comment 13•14 years ago
|
||
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•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Assignee | ||
Comment 15•14 years ago
|
||
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
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•