Closed Bug 599809 Opened 14 years ago Closed 14 years ago

mailnews/Makefile points to wrong topsrcdir, using m-c instead of c-c

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a1

People

(Reporter: bugzilla, Assigned: Callek)

References

Details

(Whiteboard: workaround #c3, str #c11)

Attachments

(2 files, 1 obsolete file)

from irc:
08:40 <%Callek> whats going on is that the |parent| makefile (in this case a m-c makefile, since we switched to libxul) sees that Makefile.in
                changed, and tries to regen it
08:40 <%Callek> it does, but with its $topsrcdir
08:40 <%Callek> and that causes us to fail.
Summary: mailnews/Makefile points to wrong top_srcdir → mailnews/Makefile points to wrong topsrcdir, using m-c instead of c-c
Attached file mailnews/Makefile
So, is this related to bug 599653 which was backed out or what? Please add some information to this bug as to *what* is failing *where*. Even if Callek and you know that, the rest of us cannot tell from what's here...
As far as we could tell, the backed out bugs only showed the problem and have not caused it. Bug 599653 lead to regenerating the Makefile in $objdir/mailnews/ which resulted in picking up the wrong topsrcdir somehow.

Workaround:

1. delete $objdir/mailnews/Makefile
2. touch configure.in (to force a reconfigure)
3. run make -f client.mk build
Whiteboard: workaround #c3
And the error message during mailnews build is:

/builds/slave/comm-central-trunk-linux-debug/build/objdir/config/nsinstall -R /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/abSetup.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/alertTestUtils.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/asyncTestUtils.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/filterTestUtils.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/folderEventLogHelper.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/IMAPpump.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/logHelper.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/mailDirService.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/mailShutdown.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/mailTestUtils.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/messageGenerator.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/messageInjection.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/messageModifier.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/msgFolderListenerSetup.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/POP3pump.js /builds/slave/comm-central-trunk-linux-debug/build/mozilla/../mailnews/test/resources/searchTestUtils.js ../mozilla/_tests/xpcshell/mailnews/resources
make[5]: /builds/slave/comm-central-trunk-linux-debug/build/objdir/config/nsinstall: Command not found
NEXT ERROR make[5]: *** [libs] Error 127
make[5]: Leaving directory `/builds/slave/comm-central-trunk-linux-debug/build/objdir/mailnews'
NEXT ERROR make[4]: *** [libs_tier_platform] Error 2
make[4]: Leaving directory `/builds/slave/comm-central-trunk-linux-debug/build/objdir/mozilla'
NEXT ERROR make[3]: *** [tier_platform] Error 2
make[3]: Leaving directory `/builds/slave/comm-central-trunk-linux-debug/build/objdir/mozilla'
make[2]: *** [default] Error 2
make[2]: Leaving directory `/builds/slave/comm-central-trunk-linux-debug/build/objdir/mozilla'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/builds/slave/comm-central-trunk-linux-debug/build/objdir'
make: *** [build] Error 2

Ok, I hope I didn't forget any more helpful information.
Looks like for now, this needs us to clobber or apply the workaround on our build boxes and do the same again once bug 599653 lands again, which is annoying work. :(

For Linux, I will look into bug 599530 today anyhow, probably, which will need a full clobber anyhow, so that should take care of it, but the other will need additional manual action just for this problem.
My current thoughts are that this is going to be hard to fix. The problem is that the makefiles need to be generated from the comm-central directory, but because configure hasn't changed, are being generated from mozilla-central and obviously failing due to the topsrcdir - as shown in comment 0.

Firefox, where we stole the idea from, wouldn't have had this problem because their makefiles wouldn't be referenced in different build systems.

The only workaround I can think of at the moment is to land changes that affect something that causes makefiles to re-generate at the same time as landing changes to any of the makefiles that we currently have built into libxul.

The longer term fix would be either to put the comm-central directories back under the mozilla-central ones, removing the need for two duplicate build systems, or implement static libxul, where we get a static libxul library and link it into one of our component libraries.
(In reply to comment #6)
> or implement static libxul, where we get a static libxul library and
> link it into one of our component libraries.

(which is something I'd like to move towards anyway).
> The longer term fix would be either to put the comm-central directories back
> under the mozilla-central ones, removing the need for two duplicate build
> systems, or implement static libxul, where we get a static libxul library and
> link it into one of our component libraries.

By this you mean putting mailnews/ into the same directory as toolkit/ lives in, right?   That's what Fennec does, truth be told I've really never understood why c-c doesn't do that.
(In reply to comment #8)
> By this you mean putting mailnews/ into the same directory as toolkit/ lives
> in, right?   That's what Fennec does, truth be told I've really never
> understood why c-c doesn't do that.

The problem is that we can't (or couldn't at the time we generated the repo) have two hg repos be based in the same local directory and have some files/subdirectories come from one and some from the other. Even if there might be some hacky technology invented for that, we won't be able to just switch to that as fast as we need a solution here (i.e. yesterday).


BTW, just an idea, can we help there with putting |ifdef COMM_BUILD| switches in the affected files in some form? Or do we have to / can we hack the script that we run for replacing the topsrcdir var and others?
a dirty hack could be to create a dep-list of Makefile.in's manually in the topsrc/Makefile.in of c-c and let the build system gen the makefiles from there.

Its ugly and could very well do extra (unneeded) work, but it should work.
For anybody who wants to have a closer look at it, the easiest step to reproduce is doing a "touch mailnews/Makefile.in" and build c-c afterwards.
Whiteboard: workaround #c3 → workaround #c3, str #c11
No longer blocks: 599653
Attached patch v1 (obsolete) — Splinter Review
Untested so far, but everything I see this should work.

One caveat, i did not take the time to magically filter out the xpfe/ autocomplete Makefile from this list, though that is the -only- m-c based directory we tie into libxul.

That dir should be moving to comm-central in the near future anyway, and negate the problem this patch poses for that, but all-in-all I think we'll be better off with this patch than without it.
Assignee: nobody → bugspam.Callek
Status: NEW → ASSIGNED
Attachment #478995 - Flags: review?(bugzilla)
The comment is a bit vague, as it doesn't tell someone looking at it why we really do that. Could you just reference something like "workaround for bug 599809" there or so?
Attached patch v2Splinter Review
Tested, and without the mistake that was caused by Neil telling me bridge.mk was included by build.mk and included here, when in reality bridge.mk was ONLY included here if build.mk was already.

Though I now have to define a var to wrap here, so we don't double the APP_* stuff here, go figure... But this is still probably the best way.
Attachment #478995 - Attachment is obsolete: true
Attachment #478995 - Flags: review?(bugzilla)
Attachment #479294 - Flags: review?(bugzilla)
Attachment #479294 - Flags: review?(kairo)
Keywords: dev-doc-needed
Keywords: dev-doc-needed
Comment on attachment 479294 [details] [diff] [review]
v2

>+# workaround Bug 599809 by making these makefiles be generated here

nit: lower-case the "Bug".

I can't take any more look into this for a couple of days, but the general approach is, while (necessarily) hacky, OK by me. If Mark gives an r+, just go for it.
Comment on attachment 479294 [details] [diff] [review]
v2

I'm not sure this is perfect, but it should help for now.
Attachment #479294 - Flags: review?(kairo)
Attachment #479294 - Flags: review?(bugzilla)
Attachment #479294 - Flags: review+
(In reply to comment #14)
> Created attachment 479294 [details] [diff] [review]
> v2
The patch looks good for me. The issues I found during testing were the following:

1) Failure due to wrong topsrcdir in mozilla/js/src/Makefile, this isn't due to this patch but perhaps a similar error. Filed bug 600520 for it.
2) mozilla/xpfe/components/autocomplete/Makefile picks up the wrong topsrcdir if the respective Makefile.in is touched, like said in comment #12
3) directory/c-sdk/Makefile has the same issue as 2), topsrcdir should be /path/to/comm-central/directory/c-sdk but is /path/to/comm-central
http://hg.mozilla.org/comm-central/rev/57c6179c357b

I might be able to come up with a "real" fix to this issue, but thats not on my immediate plate, I'm also not planning to tackle the other errors this can cause unless it becomes a problem in practice.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Target Milestone: --- → Thunderbird 3.3a1
You need to log in before you can comment on or make changes to this bug.