Now suite has moved to be an xul app on trunk, we need to clean up /suite: - Anything that is ifndef MOZ_XUL_APP we need to remove (we also need to remove the ifdef statements and tidy up what they enclose). - There are various files/directories (e.g. contents.rdf files and suite/components directory) that will need files removing as they are no longer required. This should be quite easy to do, patches welcome. It'll save us time and make the development tree cleaner and easier to work with.

I can help with the ifdefs. ;)

we also should remove components/ now.

(In reply to comment #1) > I can help with the ifdefs. > > ;) > Excellent. When you remove the ifdefs just search for MOZ_XUL_APP in the suite directory. Anything that's not MOZ_XUL_APP can come out. If it references directories then (e.g. components) then all the files within that need to be removed as well. In jar.mn files there'll be references to files that are no longer required as well. There's several ways to do it, but if you could provide a patch and a list of files that'll need removal, that would be great :-)

(In reply to comment #3) > Excellent. When you remove the ifdefs just search for MOZ_XUL_APP in the suite > directory. Anything that's not MOZ_XUL_APP can come out. If it references > directories then (e.g. components) then all the files within that need to be > removed as well. In jar.mn files there'll be references to files that are no > longer required as well. I'll be in front of my tree in a few hours (I'm on the college right now). My idea is, actually, to use a script to process the files as if MOZ_XUL_APP was set to 1. I can change it to list the files referenced in jar.mn's, on the ifndef MOZ_XUL_APP part. > There's several ways to do it, but if you could provide a patch and a list of > files that'll need removal, that would be great :-) The ifdef part is trivial, I can provide a patch soon. To list all files maybe a quite hard and require manual processing, so it can take a bit longer, but I can look at it too.

Using a script assumes you have loads of that, but this isn't true, the number should be quite nice to handle. And for every line in jar.mns, you need to also remove the respective real file, so you need to process those manually anyways. I think going over those cases manually is easier than automated, actually.

(In reply to comment #5) > Using a script assumes you have loads of that, but this isn't true, the number > should be quite nice to handle. > And for every line in jar.mns, you need to also remove the respective real > file, so you need to process those manually anyways. > I think going over those cases manually is easier than automated, actually. > mmm... maybe. As I've said, I'll do it in a few hours, because I'll have a tree in front of mine and I'll be able to generate a patch. I know it's outside the scope of '/suite'(this bug), but... Should I care for things like this: http://mxr.mozilla.org/seamonkey/source/xpinstall/packager/unix/Makefile.in#58 or ifdef MOZ_XUL_APP inside of ifdef MOZ_SUITE outside of '/suite'?

(In reply to comment #6) > I know it's outside the scope of '/suite'(this bug), but... > Should I care for things like this: > http://mxr.mozilla.org/seamonkey/source/xpinstall/packager/unix/Makefile.in#58 > > or ifdef MOZ_XUL_APP inside of ifdef MOZ_SUITE outside of '/suite'? It may need doing, but just do /suite for now. Robert's got a bug on xpfe, benjamin wants to trash xpinstall anyway, and there's other parts of the tree that Camino are still using as a non-xul app. I was going to follow them up (once I got my head round some of them again) with their own bugs. I was thinking this was a good starter into getting the tidy up going whilst we're still sorting out the regressions etc.

Beginning to wonder if a new branch will happen soon, and if so, should the completion of this bug block it.

Comment on attachment 266868 [details] [diff] [review] Two unsaved files... Thanks for the patch, in general it looks good. Some comments though: DIRS = \ branding \ browser \ common \ locales \ $(NULL) -# XXX Once SeaMonkey becomes a fully fledged xul app, we can remove -# this ifdef. -ifdef MOZ_XUL_APP # if you add DIRS here, care that app is always at the bottom of the list # as it packages up the built files on mac... DIRS += \ profile \ build \ app \ $(NULL) Please make this one DIRS structure, there is no reason to keep it as two now. tier_app_dirs += \ xpfe/components/search \ xpfe/components/bookmarks \ $(NULL) -# When Suite becomes a full MOZ_XUL_APP we can remove this ifdef -ifdef MOZ_XUL_APP -tier_app_dirs += themes -endif - tier_app_dirs += suite We need to keep the themes dir, so as above, please add/move themes and suite so that they one tier_app_dirs addition with the search and bookmarks. -# XXX Once SeaMonkey becomes a fully fledged xul app, we can remove -# this ifdef. -ifdef MOZ_XUL_APP EXTRA_COMPONENTS = nsBrowserContentHandler.js ... EXTRA_COMPONENTS += nsAboutAbout.js Again, please join the EXTRA_COMPONENTS items.

Comment on attachment 266866 [details] [diff] [review] List of files to remove 1) Neither Mark nor I are super-reviewers. 2) It doesn't make sense if we independetly review only the ifdef removal or the file removal, as both need the other in turn to be complete. Directing this to Mark, as he already started with the other part of this.

Comment on attachment 266866 [details] [diff] [review] List of files to remove r=me, though I not you've got some duplication of files in here, and /mozilla/suite/components/xulappinfo/nsIXULAppInfo.idl doesn't actually exist under /suite (and shouldn't be removed anyway). If I'm checking this in then I can cope with that, if not you may want to regenerate the list.

Comment on attachment 266979 [details] [diff] [review] Addressing Mark comments Yep, that's much better. r=me. Like Robert said, I can't give sr, so redirecting to Neil (who can).

(In reply to comment #15) > (From update of attachment 266866 [details] [diff] [review]) > r=me, though I not you've got some duplication of files in here, and > /mozilla/suite/components/xulappinfo/nsIXULAppInfo.idl doesn't actually exist > under /suite (and shouldn't be removed anyway). About the duplication, I'll take a look. About the idl: -else -# XXX This should completely go away in the future but still holds the -# xulappinfo component needed as long as suite is not a fully fledged -# MOZ_XUL_APP -DIRS += components -endif I'm might to add this, but it was under the ifndef part. > If I'm checking this in then I can cope with that, if not you may want to > regenerate the list. Yeah, I'll see what caused the duplication.

(In reply to comment #17) > About the idl: > -else > -# XXX This should completely go away in the future but still holds the > -# xulappinfo component needed as long as suite is not a fully fledged > -# MOZ_XUL_APP > -DIRS += components > -endif > > I'm might to add this, but it was under the ifndef part. Yes its under the ifndef part, but you'll find the idl file is actually in toolkit and shouldn't be removed. Everything in suite/components should be removed.

I've checked in the patch and cvs removed the files on behalf of Asrail. Thanks for the patch Asrail.