Closed
Bug 648979
(ccrework)
Opened 14 years ago
Closed 7 years ago
Rework comm-central to build underneath mozilla-central
Categories
(MailNews Core :: Build Config, defect)
MailNews Core
Build Config
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1366607
People
(Reporter: standard8, Assigned: jcranmer)
References
(Depends on 1 open bug, Blocks 1 open bug, )
Details
Attachments
(4 files, 4 obsolete files)
(deleted),
text/plain
|
standard8
:
review+
|
Details |
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
glandium
:
review+
gps
:
feedback+
|
Details | Diff | Splinter Review |
Since it was set up, the comm-central build system has served us well.
However the primary issue we have with it is that it requires us to constantly port across build system changes from mozilla-central.
If we can make direct use of the mozilla-central build system, then we don't need to copy it for the comm-central apps all the time.
Therefore the proposal is that we move comm-central to be underneath mozilla-central (this does not mean that we will be inside mozilla-central). So the directory structure we have now is:
/comm-central/mailnews
/comm-central/mail
/comm-central/mozilla/accessible
/comm-central/mozilla/toolkit
etc.
where /mozilla/ is the directory where the hg clone of mozilla-central is held.
Under the new system we'd have:
/mozilla-central/accessible
/mozilla-central/toolkit
/mozilla-central/comm/mailnews
/mozilla-central/comm/mail
etc.
We'd need mozilla-central's configure and a few other items to have some additional hooks so that we could still specify options for comm-central (e.g. building with ldap), however ted and khuey agreed that it'd be a reasonable addition to do.
Reporter | ||
Comment 1•14 years ago
|
||
I have a first draft for this. I'm currently looking at getting the patches or repo published somewhere.
Reporter | ||
Comment 2•14 years ago
|
||
I can now post the said patches. Some notes before I do that:
- I currently assume that comm-central is in a sub-directory of mozilla called 'comm'.
- I've written a script which does the main bulk of replacements that are necessary. It isn't optimised and the regexps could probably be better, but it works.
- I've got an additional patch which starts getting things working - with both the script and the patch I can get Thunderbird building and running on my mac (in 64 bit mode only), and it also passes at least some of the xpcshell-tests.
- There's much more work to do, including getting SeaMonkey and Lightning building & running.
- We'll also need the hooks for configure.in options. So far the things I know that we'll need some kind of hooks for are:
** Running LDAP sub-configures and getting variables passed in.
** Getting MOZ_STATIC_MAIL_BUILD and MOZ_COMPOSER defined in the build system.
** Properly getting things like MOZ_MAIL_NEWS defined in the build system.
Reporter | ||
Comment 3•14 years ago
|
||
(oh and the builds are currently without LDAP due to the sub-configures issues).
Reporter | ||
Comment 4•14 years ago
|
||
This is the script to do all the replacements. cd into the comm-central directory and sh ./translatescript.sh
Reporter | ||
Updated•14 years ago
|
Attachment #526800 -
Attachment is patch: false
Reporter | ||
Comment 5•14 years ago
|
||
This is the patch that is generated as a result of running the replacement script.
Reporter | ||
Comment 6•14 years ago
|
||
This is the first round of manual changes.
Note: to get everything to compile, I also add the following to the mozilla-central autoconf.mk.in:
MOZ_STATIC_MAIL_BUILD = 1
MOZ_COMPOSER = 1
Comment 7•14 years ago
|
||
I wonder if it would make sense to have a clone of comm-central in your user space where you add those patches, and merge stuff in from c-c every now and then, and we can use it as a "project branch" and try to run builds of different configs and systems against to test stuff.
(In reply to comment #6)
> Note: to get everything to compile, I also add the following to the
> mozilla-central autoconf.mk.in:
>
> MOZ_STATIC_MAIL_BUILD = 1
> MOZ_COMPOSER = 1
That's something we really need to find a way for avoiding, we should not (need to) pollute mozilla trees with comm-specific code/vars.
Reporter | ||
Comment 8•14 years ago
|
||
(In reply to comment #7)
> I wonder if it would make sense to have a clone of comm-central in your user
> space where you add those patches, and merge stuff in from c-c every now and
> then, and we can use it as a "project branch" and try to run builds of
> different configs and systems against to test stuff.
Yeah, I've kinda been debating that. I'm not sure if to have a patch queue stored or a branch. I guess a branch would work for review before merge as the changes shouldn't be that big (generally I think project branches work better if the changes are reviewed as they go in - do we want to do that here?).
> (In reply to comment #6)
> > Note: to get everything to compile, I also add the following to the
> > mozilla-central autoconf.mk.in:
> >
> > MOZ_STATIC_MAIL_BUILD = 1
> > MOZ_COMPOSER = 1
>
> That's something we really need to find a way for avoiding, we should not (need
> to) pollute mozilla trees with comm-specific code/vars.
That's covered in the last point of comment 8.
Comment 9•14 years ago
|
||
(In reply to comment #8)
> Yeah, I've kinda been debating that. I'm not sure if to have a patch queue
> stored or a branch. I guess a branch would work for review before merge as the
> changes shouldn't be that big (generally I think project branches work better
> if the changes are reviewed as they go in - do we want to do that here?).
Having review of the principles should be good, though I'm not sure if review on every single line is needed, and to some degree, even review-after-the-fact works nice if a project branch is so confined to a specific purpose and the actual changes should be easy to sort out.
> That's covered in the last point of comment 8.
This is comment 8, I guess you mean comment 2, actually ;-)
Comment 10•14 years ago
|
||
Comment on attachment 526800 [details]
Replacement script
># Replace '(/mail' with '(/comm/mail' in jar.mn files
>find . -name 'jar.mn' -exec sed -i backup -E 's/(.*)\(\/mail(.*)/\1\(\/comm\/mail\2/' {} \;
Merely looking at this script, shouldn't we do this step for suite, and similar steps for calendar?
Reporter | ||
Comment 11•14 years ago
|
||
Probably, I just wasn't concentrating on getting them working at the time.
Still hoping to set up a test repo for this, just working on fixing another bug first.
Comment 12•13 years ago
|
||
Hm, sorry, I am new to this bug, and this discussion is interesting, but just a hair's width above my head.
Does all this mean that if and when this bug gets FIXED, the mozilla-central client.py will (at least when the mozconfig says we're building the suite) pull all six of the mozilla-central, comm-central, chatzilla, venkman, LDAP SDK and DOMi repositories (like the comm-central client.py does now)? Or will there still be a comm-central client.py, which will (among other things) pull mozilla-central a directory level or so above itself?
Comment 13•13 years ago
|
||
Undetermined.
So far, it seems like we'll make m-c client.py do most of what c-c client.py does. Ted was -- in theory -- ok with that plan. But first we want to get this setup working before we delve to the client.py side.
Reporter | ||
Comment 14•13 years ago
|
||
I've now created a repository where we can work on this and try and get things running correctly.
As I think it would be useful to have some easy co-ordination on this, I've created an etherpad here:
http://etherpad.mozilla.com:9000/ccrework
with notes on how to build, what work needs doing etc. Please keep up to date.
I'm proposing that build config peers can land changes without review at this time. If non-peers wish to land, then please get a general once-over from a peer so that we can check things are heading in the right direction.
Alias: ccrework
Updated•13 years ago
|
Flags: in-testsuite-
Version: unspecified → Trunk
Reporter | ||
Updated•12 years ago
|
Blocks: c-c-m-c-merge
Assignee | ||
Comment 15•12 years ago
|
||
I've successfully built a version of Thunderbird that uses the m-c build system by doing the following things:
1. Introduce lots of symlinks everywhere, to avoid sed path changes. Basically:
c-c/mozilla/comm -> c-c
c-c/mozilla/mozilla -> c-c/mozilla
c-c/mozilla/{mail,mailnews,chat} - c-c/*
c-c/mozilla/editor/ui -> c-c/editor/ui
2. Fix paths in bridge/bridge.mozbuild, mail/app.mozbuild, and mail/configure.in
3. Flatten everything in mail/app.mozbuild
4. Disable mozbuild-migration checks in the m-c build system, as well as the xpccheck stuff
5. s/$(DIR_INSTALL)/$(INSTALL)/g in comm-central.
6. Added MOZDEPTH/MOZILLA_SRCDIR to mozilla-central/config/baseconfig.mk
From the results of this, after bug 846540 lands, the following things would need to be done:
1. Change DIR_INSTALL to INSTALL in the c-c build system (this can be done before)
2. Find some way to make xpcshell and mozmill tests work again. For xpcshell, we may have to push on making a non-broken global manifest system (bug 844655 disavows fixing that as a goal right now). For mozmill, we at least need to integrate it into top-level make/mach, and make package-tests as well.
3. Fix mail/app.mozbuild and suite/app.mozbuild, as well as the respective configure.ins, to get paths right and eliminate the COMM_BUILD duality. We can probably eliminate bridge/bridge.mozbuild at that time too, but it's not a big deal.
4. Complete all pending mozbuild migration work.
5. Break out a sed script and rewrite all critical pseudo-absolute paths.
6. Change all the builders and release engineering scripts.
I've done no testing other than make, make package-tests, and running thunderbird to see if it keels over dead so far, so it's quite possible we will have issues with things like l10n-repacks or packaging scripts. I'll let someone who knows that stuff better than I worry about those changes.
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 16•11 years ago
|
||
Status update for people following along at home:
My current patch-queue is capable of making TB (without Lightning) build under mozilla-central. The main patches that can be done before the move have been uploaded and awaiting review (basically, more moz.build migration and some makefile cleanup).
The stuff that can't be done before the move are the massive path-rewriting, which is this:
find -name Makefile.in | grep -v mozilla | grep -v ldap/sdks | xargs sed -e '/include/!s+$(topsrcdir)+$(topsrcdir)/comm+g' -i
find -name Makefile.in | grep -v mozilla | grep -v ldap/sdks | xargs sed -e 's+$(topsrcdir)/comm/mozilla+$(topsrcdir)+g' -i
find -name Makefile.in | grep -v mozilla | grep -v ldap/sdks | xargs sed -e '/config\/autoconf.mk/!s+$(DEPTH)/\([^$]\)+$(DEPTH)/comm/\1+g' -i
find -name Makefile.in | grep -v mozilla | grep -v ldap/sdks | xargs sed -e 's+$(MOZDEPTH)+$(DEPTH)+g' -i
find -name Makefile.in | grep -v mozilla | grep -v ldap/sdks | xargs sed -e 's+$(MOZILLA_SRCDIR)+$(topsrcdir)+g' -i
find -name jar.mn | grep -v mozilla | grep -v ldap/sdks | xargs sed -e 's+(/+(/comm/+' -i
find -name jar.mn | grep -v mozilla | grep -v ldap/sdks | xargs sed -e 's+(/comm/mozilla+(/mozilla+' -i
find -name moz.build | grep -v mozilla | xargs sed -e 's+$(topsrcdir)+$(topsrcdir)/comm+g' -i
Then there are several manual path changes to account for things not easily findable (like configure.in and bridge.mozbuild changes).
The final step is to kill COMM_BUILD.
As of right now, Thunderbird builds, but package-tests and l10n-check break, as do all of the comm-central specific test files.
Comment 17•11 years ago
|
||
Any relative obj dirs, e.g. MOZ_OBJDIR=@TOPSRCDIR@/../opt/ or just /opt/, are broken for me. Folks on #maildev told me that this bug fixes it.
Workaround: absolute paths work, but they are cumbersome when you have many trees.
Severity: normal → blocker
Assignee | ||
Comment 18•11 years ago
|
||
This is the final script I'll be using (minus applying any patches named temp-*.diff). Several of the steps are autogenerated by running commands instead of manually writing patches that will tend to get obsolete. I've separated out each step into different commits so that it's clear what all the changes have in common, but this does break pretty severely the rule that individual patches should work on their own.
Assignee | ||
Comment 19•11 years ago
|
||
This is the file named rewrite-paths.diff mentioned in the merge script. It contains the manual changes not covered by the automatic rewrite, as well as moving the bridge.mozbuild file to mailnews.mozbuild to eliminate a useless and potentially confusing top-level directory.
Something else that could be done is to move the mork logic under mailnews/db, but since m-c already has a top-level db/ directory, I don't think it's quite so necessary for the merge work.
Attachment #526800 -
Attachment is obsolete: true
Attachment #526803 -
Attachment is obsolete: true
Attachment #526805 -
Attachment is obsolete: true
Attachment #8376652 -
Flags: review?(mbanner)
Assignee | ||
Comment 20•11 years ago
|
||
This is the package-tests.diff file, which mostly contains manual fixes to get packaging to work properly, most of which cannot be applied prior to this bug because it breaks under the current build system.
Attachment #8376653 -
Flags: review?(mbanner)
Assignee | ||
Comment 21•11 years ago
|
||
This is m-c-post.diff, the changes we need to mozilla-central to make this work. I'm not asking for review yet because I'm not sure whom to ask, and I also expect this code to be more likely to bitrot than the other files. Effectively, we need a dependency to make the pseudo-derecurse work--this is the default in m-c and will I believe eventually be the only supported mode; it was never ported to c-c because both glandium and I gave up trying to figure out why it wasn't working.
The OS X universal build changes are ugly, but after discussing with Ted in #build, we came to the conclusion that adding yet-another-directory to the list was the way to go.
Assignee | ||
Comment 22•11 years ago
|
||
In terms of testing:
The Thunderbird code is tested by periodic pushes to Alder acting as a try server for this bug. This also confirms the buildability of Lightning for TB, although we don't run any Lightning tests on buildbot.
I've manually built SeaMonkey on one of the attempted heads (after dropping inspector, venkman, and chatzilla in their appropriate places), and that worked, and I've built it on the latest change without the three extensions, and that worked as well. Instantbird doesn't build for me on Linux with comm-central, but it builds just as well on the merged repository, so I'll take that as a sign that it should work.
The manual builds are done as regular ./mach builds only on Linux. My experience with build system work is that mailnews, calendar, and mail are all far nastier in stressing the build system than chat, im, and suite, so I feel safe in assuming that works-on-Linux for SM and IB means works-on-all-platforms. I'm not so confident that packaging works properly for SM and IB, but I assume that packaging issues are not quite so high a priority with both IB and SM's buildbots being down and they don't preclude people working on code locally. I'll be happy to work with people to fix build errors introduced by this change.
I haven't tried Firefox on it yet, but I will be sure to do a real try run before getting review on m-c-post.diff and certainly before pushing anything to m-c. :-)
Comment 23•11 years ago
|
||
Comment on attachment 8376654 [details] [diff] [review]
m-c-post.diff
I think you should ask gps for review on the m-c bit as he is the build system owner.
Comment 24•11 years ago
|
||
Comment on attachment 8376654 [details] [diff] [review]
m-c-post.diff
Review of attachment 8376654 [details] [diff] [review]:
-----------------------------------------------------------------
::: Makefile.in
@@ +294,5 @@
> ifdef ENABLE_CLANG_PLUGIN
> js/src/export config/export: build/clang-plugin/export
> endif
> +ifdef MOZ_LDAP_XPCOM
> +ldap/export: config/export
This shouldn't be needed. The backend makes */export depend on config/export already.
Reporter | ||
Comment 25•11 years ago
|
||
Comment on attachment 8376275 [details]
merge-script.sh
I've not tested it, but it looks fine.
Attachment #8376275 -
Flags: review?(standard8) → review+
Reporter | ||
Updated•11 years ago
|
Attachment #8376652 -
Flags: review?(standard8) → review+
Reporter | ||
Updated•11 years ago
|
Attachment #8376653 -
Flags: review?(standard8) → review+
Reporter | ||
Comment 26•11 years ago
|
||
Patches look good, though obviously we need to move forward with the discussions about when we can land this etc.
Assignee | ||
Comment 27•11 years ago
|
||
This is an updated version of the m-c-post.diff patch taking into account glandium's prior comment. I'm also considering this review a chance to have m-c build system people weigh on the patches here.
It passes both alder and try (https://tbpl.mozilla.org/?tree=Try&rev=c5b6d5e4ee1a).
Attachment #8376654 -
Attachment is obsolete: true
Attachment #8383181 -
Flags: review?(mh+mozilla)
Attachment #8383181 -
Flags: feedback?(gps)
Updated•11 years ago
|
Attachment #8383181 -
Flags: review?(mh+mozilla) → review+
Updated•11 years ago
|
Attachment #8383181 -
Flags: feedback?(gps) → feedback+
Updated•7 years ago
|
Updated•7 years ago
|
Depends on: comm-taskcluster
Updated•7 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 28•7 years ago
|
||
So in what way is this fixed?
You need to log in
before you can comment on or make changes to this bug.
Description
•