1045947 - Port |Bug 1045484 - TOOL_DIRS, TEST_TOOL_DIRS and PARALLEL_DIRS are no more| to Instantbird

Status: RESOLVED WONTFIX

(Instantbird Graveyard :: Other, defect)

Description

[Quentin Headen [:qheaden]]

Currently, the mach build system cannot get past the configure step when building Instantbird. It complains that the moz.build files are using PARALLEL_DIRS instead of DIRS.

Comment 1

[Quentin Headen [:qheaden]]

Attachment: Patch 1

This patch fixes configuration issues with Instantbird. As for the rest of the codebase, I'm sure bug 1045762 can take care of that.

Comment 2

[Patrick Cloke [:clokep]]

It looks like we'd also need fixes for purple. Have you tested building with that?

Comment 3

[Quentin Headen [:qheaden]]

Attachment: Patch 1 Purple Extension

I made a separate patch for the purple extension since it is in a separate repository. This fixes the moz.build stuff in there.

Comment 4

[Quentin Headen [:qheaden]]

Just for reference, the purple patch is to be applied against this repo: http://hg.mozilla.org/users/florian_queze.net/purple

Comment 5

[Florian Quèze [:florian]]

Comment on attachment 8464411 [details] [diff] [review]
Patch 1

"app" should stay at the end. And I would like "windows/installer" to stay at the end too, like I said in bug 1045484 comment 2.

Comment 6

[Patrick Cloke [:clokep]]

I reviewed a patch from Florian over IRC that I'll push soon.

10:01:11 <clokep> flo-retina: So that patch looks reasonable, but I'd like to vaguely understand it before pushing. :)
10:01:42 <flo-retina> how can I ensure your understanding stays vague? :-P
10:03:11 <clokep> I'm pretty sure it's guaranteed. ;)
10:03:28 <flo-retina> ah!
10:03:37 <flo-retina> feel free to ask questions :)
10:06:53 <flo-retina> clokep: which part do you not understand enough? :)
10:07:19 <clokep> flo-retina: Well, I haen't looked this morning but last night I wasn't understanding the move of prpls.
10:08:01 <flo-retina> you mean moving around some of the code related to building the dynamic prpls?
10:08:09 <clokep> I believe so, yes.
10:09:03 <flo-retina> before we were building the static prpls in the PARALLEL_DIRS variable of the libpurple folder, and the dynamic prpls from the purplexpcom folder
10:09:16 <flo-retina> the reason for that was that purplexpcom needs to be built before we can link the dynamic prpls
10:10:25 <flo-retina> so the existing split between the two was a hack
10:11:03 <flo-retina> that hack no longer works, because the build system doesn't take into account the order of things anymore, and computes an "optimal" order itself by looking at what library needs what to be linked.
10:11:38 <flo-retina> so moving stuff around is actually just removing the previous hack that no longer works
10:12:17 <clokep> I see.
10:12:20 <flo-retina> then I added a new hack to enforce that libpurplexpcom.dylib is built before we link dynamic prpls. For that I made xpcomModule.o depend on libpurplexpcom.dylib.
10:13:12 <clokep> flo-retina: That's createing xpcomModule.cpp with PUPLREXPCOM_LIBS?
10:13:14 <flo-retina> that's arbitrary, and I would have preferred to just make target:: depend on libpurplexpcom.lib... except that gets executed in parallel, which means that libpurplexpcom.dylib is guaranteed to exist when we leave the folder, but not when we attempt to link libnull.dylib
10:13:39 <flo-retina> clokep: that question doesn't make sense
10:13:52 <clokep> flo-retina: Line 48 of http://pastebin.instantbird.com/783759
10:14:15 <flo-retina> clokep: in gmake syntax, |foo: bar boo| means that bar and boo need to exist and be older than foo if foo already exists
10:14:27 <flo-retina> if not, the rules for bar and boo run before the rule for foo
10:16:13 <clokep> Yes.
10:16:44 <flo-retina> of course, I would have preferred if I could just have made this clean, instead of adding a new hack. The clean solution would be to add "purplexpcom" in the USE_LIBS list in prpl.py for dynamic prpls. But that doesn't work, because moz.build doesn't want the same library to be an xpcom component and a dynamic library :(
10:17:06 <clokep> Right. :-\
10:17:23 <clokep> So is this still linking purplexpcom every time like you mentioned on IRC?
10:17:45 <flo-retina> it's doing it once for each dynamic prpl
10:18:04 <aleth> I wonder if some of that should be in a comment, for the next time this breaks ;)
10:18:24 <flo-retina> maybe only because we build several dynamic prpls in parallel. If we had finished linking the first one before linking the second one, I think we would link purplexpcom only once.
10:19:01 <flo-retina> aleth: I wondered if that should be in a bug, to request the changes so that we could just USE_LIBS += ['purplexpcom'] and avoid the hackery
10:19:34 <aleth> flo-retina: That would be even better of course! Maybe glandium would take pity on us
10:19:36 <flo-retina> but at this point I would prefer to have the attention of build peers directed to the issues we can't workaround rather than to the ugliness we can workaround
10:19:46 <aleth> yeah...
10:19:50 <clokep> flo-retina: So I think the patch looks good, I definitely think we should comment either in the code or in a bug.
10:20:09 <flo-retina> clokep: I also think we should make it work on Windows
10:21:24 <flo-retina> clokep: the likely problem for Windows is this hack: https://hg.mozilla.org/users/florian_queze.net/purple/file/8e825c68b4a7/purplexpcom/src/Makefile.in#l86
10:21:29 <flo-retina> I'm pretty sure it's now broken.
10:23:20 <clokep> The moving of the library?
10:24:10 <flo-retina> the rename of the *fake* import library to the final library name, in time so that we can link against it
10:24:45 <flo-retina> this is again a hack because we are not supposed to link against xpcom components :-/
10:25:16 <clokep> :(
10:26:05 <flo-retina> statically linking libpurple into purplexpcom has always been a hack :)
10:26:33 <clokep> I see.
10:26:45 <clokep> Is this something I should create a build for to test w/?
10:26:46 <flo-retina> but a hack that overall seems better than the previous hack, which was a fake xpcom component that was forcing a load of the libpurple binary and then forcing the real purplexpcom and forwarding the calls to it...

Comment 7

[Patrick Cloke [:clokep]]

http://hg.mozilla.org/users/florian_queze.net/purple/rev/9962cc44aded

I believe at this point we should have working builds on Linux, but Windows and Mac will need some more work.

Florian, any thoughts on whether we do this in a new bug or here?

Comment 8

[Sylvestre Ledru [:Sylvestre]]

On the behalf of Florian:
Closing bugs related to the Instantbird UI as WONTFIX, as the development of the standalone chat client Instantbird has stopped. Instantbird users are encouraged to migrate to Thunderbird. The user interface of instant messaging in Thunderbird will feel familiar, as the Thunderbird IM support started as a fork of Instantbird.