Last Comment Bug 662160 - ipccode does not build on gecko >= 5.0
: ipccode does not build on gecko >= 5.0
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: 5 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Patrick Brunschwig
:
Mentors:
Depends on:
Blocks: ipc
  Show dependency treegraph
 
Reported: 2011-06-05 09:13 PDT by Patrick Brunschwig
Modified: 2011-06-08 01:16 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to fix. (9.48 KB, text/plain)
2011-06-05 09:13 PDT, Patrick Brunschwig
cjones.bugs: review+
ajvincent: feedback+
Details

Description Patrick Brunschwig 2011-06-05 09:13:21 PDT
Created attachment 537457 [details]
Patch to fix.

ipccode still uses the old way of mutex-locking which is no longer supported.

The attached patch also adds a readme.txt containing instructions for building.
Comment 1 Alex Vincent [:WeirdAl] 2011-06-05 11:48:32 PDT
I want to think twice about this one.  It looks like you're completely removing support for FF3.x (pre-Gecko 2.0) builds.

Given the number of places you had to change the old way to the new one, may I suggest you branch (or tag, if you don't think we'll need to ever change it) the existing code for pre-2.0 platform consumers, and land this on tip.

Is there any way in the configure step that we can check the platform version we're building?  That would allow us to fail much faster than if someone accidentally checked out tip code against FF3.6, for example.

I'll finish my review later today.  I'm not up to speed on mozilla::Mutex, and I typically build using --enable-extensions (which is why I asked about checking in configure).
Comment 2 Alex Vincent [:WeirdAl] 2011-06-05 21:10:07 PDT
Comment on attachment 537457 [details]
Patch to fix.

I'm sorry, but I really don't think I can honestly review this one.  I don't know how our mutexes work.

I'd recommend getting review from someone on this list:
http://hg.mozilla.org/mozilla-central/log/068197c2a88e/xpcom/glue/Mutex.h

feedback+ with your response to my previous comment about branching/tagging and configure-time failure:  nothing obviously broken here.

I'm hoping cjones can help us out here.

>--- /dev/null	2011-06-05 17:34:47.000000000 +0200
>+++ readme.txt	2011-06-05 18:08:42.000000000 +0200
>+makemake tries to read your .mozconfig file to determint @MOZ_OBJDIR@. You can
>+alternatively provide your own MOZ_OBJDIR using makemake -o

typo:  determine
Comment 3 Patrick Brunschwig 2011-06-06 11:22:10 PDT
(In reply to comment #1)
> I want to think twice about this one.  It looks like you're completely
> removing support for FF3.x (pre-Gecko 2.0) builds.

Yes indeed. Those who still want the old code can get it under the tag "gecko-192."

> Is there any way in the configure step that we can check the platform
> version we're building?  That would allow us to fail much faster than if
> someone accidentally checked out tip code against FF3.6, for example.

I don't think this is really needed. IF the build fails, you checkout the new code and just rebuild ipccode, you don't need to re-build the rest.

> I'll finish my review later today.  I'm not up to speed on mozilla::Mutex,
> and I typically build using --enable-extensions (which is why I asked about
> checking in configure).

I found that --enable-extensions don't work on Thunderbird, that's why I wrote makemake.

(In reply to comment #2)
> Comment on attachment 537457 [details]
> Patch to fix.
> 
> I'm sorry, but I really don't think I can honestly review this one.  I don't
> know how our mutexes work.

No worries. It's relatively simple though. There's a page describing the changes you'll have to follow to move from Gecko 2.0 to Gecko 5.0:
https://developer.mozilla.org/En/XPCOM_Thread_Synchronization
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-06-06 13:37:55 PDT
The PRLock->Mutex conversion looks fine.  I can r+ or feedback+ that if you want, not sure what you're looking for.
Comment 5 Patrick Brunschwig 2011-06-07 10:32:38 PDT
Checked in: http://hg.mozilla.org/ipccode/rev/3aa8d8e7a70a

Note You need to log in before you can comment on or make changes to this bug.