313780 - use OS_ARCH for xpcom makefiles instead of MOZ_WIDGET_TOOLKIT

Status: NEW

(Firefox Build System :: General, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Right now xpcom/io uses the MOZ_WIDGET_TOOLKIT makefile define to determine whether to include nsILocalFileMac ans nsILocalFileOS2.  It really should be using some other define for the platform, but we don't seem to have anything appropriate.  I'm not sure if there are other places in the tree where we do this..

Comment 1

[Benjamin Smedberg]

Sure we do, it's known as OS_ARCH and it's very reliable.

Comment 2

[cls]

Unfortunately, OS_ARCH isn't sufficient for certain edge cases (e.g., X11 on Darwin/OSX builds).  Why do we need to reduce the uses of MOZ_WIDGET_TOOLKIT ?

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #2)
> Unfortunately, OS_ARCH isn't sufficient for certain edge cases (e.g., X11 on
> Darwin/OSX builds).  Why do we need to reduce the uses of MOZ_WIDGET_TOOLKIT ?

For xpcom in particular, it should be sufficient -- X11 on Darwin/OSX builds will still use Darwin NSPR/file IO/etc.

We need to remove MOZ_WIDGET_TOOLKIT for places that have no dependency on the graphical subsystem; in particular because in the near future we'll be building with the same gfx but a different widget toolkit.  Right now the only way to specify that is by adding a new --enable-default-toolkit value.

Note that the current system today would break if you tried to do a Darwin Gtk/X11 build.  Not very interesting, but it's yet another edge case.
I've got a patch for xpcom coming up shortly.

Comment 4

[Darin Fisher]

netwerk/base/src/Makefile.in has similar MOZ_WIDGET_TOOLKIT conditionals that should probably be kept in sync with xpcom/io/Makefile.in.

Comment 5

[cls]

> For xpcom in particular, it should be sufficient -- X11 on Darwin/OSX builds
> will still use Darwin NSPR/file IO/etc.

No, it will not since Darwin != OSX. Right now, if you did a X11/OSX build, it would use the default Unix code path that has no OSX-toolkit dependencies (ie, nsLocalFileUnix instead of nsLocalFileOSX).  Yes, that means X11/OSX builds use ~/.mozilla instead of ~/Library/Mozilla, among other changes.

If you changed that to be simply based upon OS_ARCH, then X11/Darwin builds will break because of dependencies upon Carbon and other OSX toolkit dependencies.  We need some way to determine whether to use these additional features OS level features that were traditionally associated with the toolkit.

Sidenote: I'm sure the people trying to get X11/cygwin builds running are going to hit a similar problem on win32 with this proposed change.

Comment 6

[Benjamin Smedberg]

I'm not sure how much Darwin-not-mac should matter, and I really don't care at all about win32-gtk or win32-xlib.

Comment 7

[cls]

How can (the collective) we be so concerned about marketshare but be so ready to cut out our niche users?  Darwin-not-mac matters so much as getting the product into every possible users' hands matters.  And after the initial effort (years ago) of getting X11/Darwin working, how much effort has it been to keep it working?  Next to nothing.  And all you have to do to maintain that miniscule userbase is not break the build when informed...especially over a bad assumption.  If only all of our non-tier1 platforms were so lucky.

I think we could get away with simply replacing the MOZ_WIDGET_TOOLKIT tests with a MOZ_X11 test with the assumption that MOZ_X11 == Unix-like code path.

Btw, my xlib/OSX build from last week works fine.  I'm guessing the assertion that the Darwin gtk build would break comes from dependency issues?

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: xpcom patch bits

Attaching this here in case someone either wants to continue working on this or if I have time later on; this is most of xpcoms's makefiles fixed.  darwin-not-mac should probably be fixed by changing the current Darwin value of OS_ARCH to really be OSX, and letting Darwin mean just Darwin, if we really wanted to fix that.

Comment 9

[Gregory Szorc [:gps]]

It's been 11 years since activity on this bug, but I think the old comments still apply. What I'm not sure is if they are still relevant.

glandium: could you please spend no more than 5 minutes triaging this for relevance?

Comment 10

[Mike Hommey [:glandium]]

This, sadly, still applies.