Closed Bug 563605 Opened 10 years ago Closed 9 years ago

All platforms should support IPC on 2.0

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b4

People

(Reporter: glandium, Assigned: glandium)

Details

Attachments

(1 file, 4 obsolete files)

Attached patch Use a whitelist to disable ipc (obsolete) — Splinter Review
The current approach with a blacklist makes the build fail on anything that is not linux, osx or windows AND x86, x86-64 or armel. Which means failures are to be expected on any sparc, hppa, alpha, mips, etc. builds, or on x86{,-64} BSDs.

IPC should be enabled based on a whitelist instead of the opposite.
Comment on attachment 443303 [details] [diff] [review]
Use a whitelist to disable ipc

As I told on IRC, this implements a whitelist instead of a blacklist. Please note that on trunk, ipc is disabled for ppc darwin, while on 1.9.2, it is disabled for any darwin. This means the equivalent patch for 1.9.2 would need to leave darwin out (except if the difference is unintended).
Attachment #443303 - Attachment is patch: true
Attachment #443303 - Attachment mime type: application/octet-stream → text/plain
Attachment #443303 - Flags: review?(benjamin)
After discussion on IRC: we think we'd like to require all platforms to have IPC for 1.9.3 (Firefox 4), in order to enable multi-process jetpack at least. Therefore I would only accept a branch patch of this sort. And yes, the darwin difference between trunk and branch is intentional.
Fair enough. Here is a 1.9.2 version that I think should do the right thing.
Assignee: nobody → mh+mozilla
Attachment #443303 - Attachment is obsolete: true
Attachment #443563 - Flags: review?(benjamin)
Attachment #443303 - Flags: review?(benjamin)
Attached patch Proposed change for trunk (obsolete) — Splinter Review
Since you suggest all platforms should support IPC on 1.9.3, it's probably time to remove the blacklist on m-c.
Attachment #443564 - Flags: review?(benjamin)
Comment on attachment 443563 [details] [diff] [review]
Use a whitelist to disable ipc (1.9.2)

Looks right to me. We should probably run tryserver builds to make sure it actually does the right thing.
Attachment #443563 - Flags: review?(benjamin) → review+
Comment on attachment 443564 [details] [diff] [review]
Proposed change for trunk

We need bug 563747 before we can take this.
From a first try server run, it appears msvc* doesn't match. mingw* should. Pushed for another try.
Attachment #443563 - Attachment is obsolete: true
Attachment #443622 - Flags: review?(benjamin)
case "$target" in
*-cygwin*|*-mingw*|*-msvc*|*-mks*|*-wince|*-winmo)

is what we've used elsewhere in configure, except obviously we'd want to exclude wince/winmo
I'm not convinced all of these should be put there, especially cygwin and mks. At least not without testing them. As for msvc, I'm wondering where that would be used.
Comment on attachment 443622 [details] [diff] [review]
Use a whitelist to disable ipc (1.9.2) v2

*-cygwin*|*-mingw*|*-msvc*|*-mks* are all possible toolchains for building a windows target, oddly enough. We don't support an actual cygwin target, but we build using the cygwin shell/toolchain and MSVC as the compiler.
Attachment #443622 - Flags: review?(benjamin) → review-
Comment on attachment 443564 [details] [diff] [review]
Proposed change for trunk

This is fine, although you'll need to unbitrot it since the PPC patch landed already.
Attachment #443564 - Flags: review?(benjamin) → review+
Harmless patch, build config only
Attachment #443564 - Attachment is obsolete: true
Attachment #459029 - Flags: approval2.0?
Comment on attachment 459029 [details] [diff] [review]
Unbitrotted patch (see comments 2 and 4)

a=beltzner
Attachment #459029 - Flags: approval2.0? → approval2.0+
Summary: configure should use a whitelist to disable ipc instead of a blacklist → All platforms should support IPC on 2.0
Attachment #443622 - Attachment is obsolete: true
Attachment #459029 - Attachment description: Unbitrotted patch → Unbitrotted patch (see comments 2 and 4)
http://hg.mozilla.org/mozilla-central/rev/c99891ad5ceb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
You need to log in before you can comment on or make changes to this bug.