Closed
Bug 543976
Opened 15 years ago
Closed 15 years ago
Clean up Maemo defines
Categories
(Firefox Build System :: General, defect)
Tracking
(status1.9.2 .2-fixed)
RESOLVED
FIXED
mozilla1.9.3a2
Tracking | Status | |
---|---|---|
status1.9.2 | --- | .2-fixed |
People
(Reporter: dougt, Assigned: dougt)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ted
:
review+
beltzner
:
approval1.9.2.2+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dougt
:
review+
|
Details | Diff | Splinter Review |
We are adding support for Maemo 6 and noticed that the Maemo 5 configuration is a bit of a mess.
We are proposing that there is only one define that determines if you are running MAEMO or not:
MOZ_PLATFORM_MAEMO
If set (with ac_add_options --with-maemo-version=#), it will be the version of the MAEMO that you are targeting (Currently 5 or 6).
Two other #defines will be:
MOZ_PLATFORM_MAEMO_CFLAGS
MOZ_PLATFORM_MAEMO_LIBS
These will include the common flags needed to build and link. We think that this approach will make the maemo section of config a bit easier to understand and extend. It will also be easier as a developer to figure out which flags you need to add.
Assignee | ||
Comment 1•15 years ago
|
||
Assignee: nobody → mozbugz
Attachment #424985 -
Flags: review?
Assignee | ||
Comment 2•15 years ago
|
||
Attachment #424986 -
Flags: review?(mark.finkle)
Assignee | ||
Updated•15 years ago
|
Attachment #424985 -
Flags: review? → review?(ted.mielczarek)
Updated•15 years ago
|
Attachment #424986 -
Flags: review?(mark.finkle) → review+
Comment 3•15 years ago
|
||
Comment on attachment 424985 [details] [diff] [review]
m-c patch
>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>+dnl = Maemo checks
>+dnl ========================================================
>+
>+MOZ_ARG_WITH_STRING(maemo-version,
>+[ --with-maemo-version=MAEMO_SDK_TARGET_VER
>+ Maemo SDK Version],
>+ MAEMO_SDK_TARGET_VER=$withval)
>+
>+case "$MAEMO_SDK_TARGET_VER" in
>+5)
>+ MOZ_PLATFORM_MAEMO=5
>+ ;;
>+
>+6)
>+ MOZ_PLATFORM_MAEMO=6
>+ ;;
>+*)
>+ ;;
>+esac
You should probably error on unknown versions here.
>+
>+if test $MOZ_PLATFORM_MAEMO; then
>+ AC_DEFINE_UNQUOTED([MOZ_PLATFORM_MAEMO], $MAEMO_SDK_TARGET_VER)
Use the MOZ_PLATFORM_MAEMO variable you just defined instead of MAEMO_SDK_TARGET_VER.
>+
>+ if test -z "$MOZ_ENABLE_DBUS"; then
>+ AC_MSG_ERROR([DBus is required when building for Maemo])
>+ fi
>+
>+ MOZ_GFX_OPTIMIZE_MOBILE=1
>+ MOZ_WEBGL_GLX=
>+
>+ if test $MOZ_PLATFORM_MAEMO == 5; then
I think you want one '=' here, I believe '==' is a bashism or something.
>+ dnl if we have Xcomposite we should also have Xdamage and Xfixes
>+ AC_CHECK_HEADERS([X11/extensions/Xdamage.h], [],
>+ [AC_MSG_ERROR([Couldn't find X11/extentsions/Xdamage.h which is required for composited plugins.])])
Typo in extensions in the comment.
>+ AC_CHECK_LIB(Xcomposite, XCompositeRedirectWindow, [XCOMPOSITE_LIBS="-lXcomposite -lXdamage -lXfixes"],
>+ [MISSING_X="$MISSING_X -lXcomposite"], $XLIBS)
>+
>+ AC_SUBST(XCOMPOSITE_LIBS)
>+
>+ PKG_CHECK_MODULES(LIBHILDONMIME,libhildonmime)
>+ MOZ_PLATFORM_MAEMO_LIBS="$MOZ_PLATFORM_MAEMO_LIBS $LIBHILDONMIME_LIBS"
>+ MOZ_PLATFORM_MAEMO_CFLAGS="$MOZ_PLATFORM_MAEMO_CFLAGS $LIBHILDONMIME_CFLAGS"
>+
>+ PKG_CHECK_MODULES(LIBOSSO,libosso)
>+ MOZ_PLATFORM_MAEMO_LIBS="$MOZ_PLATFORM_MAEMO_LIBS $LIBOSSO_LIBS"
>+ MOZ_PLATFORM_MAEMO_CFLAGS="$MOZ_PLATFORM_MAEMO_CFLAGS $LIBOSSO_CFLAGS"
>+
>+ PKG_CHECK_MODULES(LIBHILDONFM,hildon-fm-2)
>+ MOZ_PLATFORM_MAEMO_LIBS="$MOZ_PLATFORM_MAEMO_LIBS $LIBHILDONFM_LIBS"
>+ MOZ_PLATFORM_MAEMO_CFLAGS="$MOZ_PLATFORM_MAEMO_CFLAGS $LIBHILDONFM_CFLAGS"
I think you are likely to want to error here if some of these libs are not found, no? If you're missing libhildonmime or libosso things aren't going to work properly, right?
>diff --git a/content/canvas/src/Makefile.in b/content/canvas/src/Makefile.in
>--- a/content/canvas/src/Makefile.in
>+++ b/content/canvas/src/Makefile.in
>@@ -54,22 +54,22 @@ CPPSRCS = \
> CanvasUtils.cpp \
> nsCanvasRenderingContext2D.cpp \
> $(NULL)
>
> # Canvas 3D Pieces
>
> ifdef MOZ_WEBGL
>
>-ifeq (1_1,$(MOZ_X11)_$(NS_OSSO))
>+ifeq (1_1,$(MOZ_X11)_$(MOZ_PLATFORM_MAEMO))
These checks are broken, MOZ_PLATFORM_MAEMO is always going to be '5' or '6' as you've defined it.
r- for those few things but overall it looks pretty good.
Attachment #424985 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 4•15 years ago
|
||
Attachment #424985 -
Attachment is obsolete: true
Attachment #425922 -
Flags: review?
Assignee | ||
Updated•15 years ago
|
Attachment #425922 -
Flags: review? → review?(ted.mielczarek)
Updated•15 years ago
|
Attachment #425922 -
Flags: review?(ted.mielczarek) → review+
Comment 5•15 years ago
|
||
Comment on attachment 425922 [details] [diff] [review]
m-c patch v.2
[Checkin: Comment 6 & 8]
>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>+MOZ_ARG_WITH_STRING(maemo-version,
>+[ --with-maemo-version=MAEMO_SDK_TARGET_VER
>+ Maemo SDK Version],
>+ MAEMO_SDK_TARGET_VER=$withval)
>+
>+case "$MAEMO_SDK_TARGET_VER" in
>+5)
>+ MOZ_PLATFORM_MAEMO=5
>+ ;;
>+
>+6)
>+ MOZ_PLATFORM_MAEMO=6
>+ ;;
>+
>+-1)
>+ dnl We aren't compiling for Maemo, move on.
>+ ;;
>+*)
>+ AC_MSG_ERROR([Unknown Maemo Version. Try setting --with-maemo-version to something else.])
I'd probably have this say "supported versions are 5 and 6".
Looks good otherwise, r=me
Assignee | ||
Comment 6•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/7bd24f6b3faf
waiting on m-b.
Assignee | ||
Updated•15 years ago
|
Attachment #425922 -
Flags: approval1.9.2.2?
Comment 7•15 years ago
|
||
Comment on attachment 425922 [details] [diff] [review]
m-c patch v.2
[Checkin: Comment 6 & 8]
a=beltzner, dougt assures me this is all IFDEFery.
Attachment #425922 -
Flags: approval1.9.2.2? → approval1.9.2.2+
Assignee | ||
Comment 8•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a90ba7fec2a1
when this clears, i will push m-b bits.
Comment 9•15 years ago
|
||
I'm seeing a couple of error messages in my Mac OS X build as a result of this (although they seem to be harmless, the build still completes):
/Users/jonathan/mozdev/mc/config/preprocessor.pl:/Users/jonathan/mozdev/mc/config/system-headers:1012: error evaluating if: invalid argument: '(MOZ_PLATFORM_MAEMO == 5)'
/Users/jonathan/mozdev/mc/js/src/config/preprocessor.pl:/Users/jonathan/mozdev/mc/js/src/config/system-headers:1012: error evaluating if: invalid argument: '(MOZ_PLATFORM_MAEMO == 5)'
Assignee | ||
Comment 10•15 years ago
|
||
it looks like it is only perl that is getting confused. I am using 5.8.9, but do not see this problem. Its an easy fix to this (#if defined(MOZ_PLATFORM_MAEMO) && (MOZ_PLATFORM_MAEMO == 5)). But I like to understand why you are seeing this and I am not.
Assignee | ||
Comment 11•15 years ago
|
||
http://hg.mozilla.org/mobile-browser/rev/e716e0c7768b
Leaving open to resolve jonathan's error.
Comment 12•15 years ago
|
||
After some experimentation, it seems that preprocessor.pl does not allow parens or spaces in the argument to #if. Removing them eliminates the error messages.
(This was seen when building for i386 on OS X 10.6; possibly preprocessor.pl is only used here in cross-compilation setups?)
Attachment #426128 -
Flags: review?(mozbugz)
Comment 13•15 years ago
|
||
(In reply to comment #12)
I'm using Fedora 12, x86_64
After comment #11, I got a compile error (like bug 402892 comment #57) with "--enable-gio" in configure script.
And attachment 426128 [details] [diff] [review] fix it.
Assignee | ||
Comment 14•15 years ago
|
||
Jonathan -- thank you:
http://hg.mozilla.org/mozilla-central/rev/c96143454881
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2e21cc41bb66
Matsuura - can you update and verify this fixes the compile error?
Comment 15•15 years ago
|
||
Yes.
The compile error now fixed.
Assignee | ||
Updated•15 years ago
|
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
status1.9.2:
--- → .2-fixed
Assignee | ||
Updated•15 years ago
|
Attachment #426128 -
Flags: review?(mozbugz) → review+
Updated•15 years ago
|
Attachment #426128 -
Attachment description: clean up #if arguments to eliminate preprocessor.pl error → clean up #if arguments to eliminate preprocessor.pl error
[Checkin: Comment 14]
Updated•15 years ago
|
Attachment #425922 -
Attachment description: m-c patch v.2 → m-c patch v.2
[Checkin: Comment 6 & 8]
Updated•15 years ago
|
Attachment #424986 -
Attachment description: m-b patch → m-b patch
[Checkin: Comment 11]
Updated•15 years ago
|
Target Milestone: --- → mozilla1.9.3a2
Version: unspecified → Trunk
Updated•15 years ago
|
Blocks: C192ConfSync
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•