543976 - Clean up Maemo defines

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Doug Turner (:dougt)]

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.

Comment 1

[Doug Turner (:dougt)]

Attachment: m-c patch

Comment 2

[Doug Turner (:dougt)]

Attachment: m-b patch [Checkin: Comment 11]

Comment 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[Doug Turner (:dougt)]

Attachment: m-c patch v.2 [Checkin: Comment 6 & 8]

Comment 5

[(not currently active) Ted Mielczarek]

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

Comment 6

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/7bd24f6b3faf

waiting on m-b.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

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.

Comment 8

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a90ba7fec2a1

when this clears, i will push m-b bits.

Comment 9

[Jonathan Kew [:jfkthame]]

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)'

Comment 10

[Doug Turner (:dougt)]

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.

Comment 11

[Doug Turner (:dougt)]

http://hg.mozilla.org/mobile-browser/rev/e716e0c7768b

Leaving open to resolve jonathan's error.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: clean up #if arguments to eliminate preprocessor.pl error [Checkin: Comment 14]

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?)

Comment 13

[Takanori MATSUURA]

(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.

Comment 14

[Doug Turner (:dougt)]

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

[Takanori MATSUURA]

Yes.
The compile error now fixed.