Closed Bug 464394 Opened 12 years ago Closed 12 years ago

Define MOZ_GFX_OPTIMIZE_MOBILE on WINCE

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: dougt, Assigned: dougt)

References

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 3 obsolete files)

No description provided.
Attached patch patch v.1 (obsolete) — Splinter Review
Attachment #347695 - Flags: review?(ted.mielczarek)
Mind adding a --enable-mobile-optimize flag or something?  Would be nice to be able to build desktop builds with the same stuff.  (Also, fix the comment there to not mention OSSO, since it's the WINCE section :)
Attachment #347695 - Flags: review?(ted.mielczarek) → review-
Version: psm1.01 → Trunk
Attached patch patch v.2 (obsolete) — Splinter Review
Attachment #347695 - Attachment is obsolete: true
Attachment #355840 - Flags: superreview?(ted.mielczarek)
Attachment #355840 - Flags: review?(vladimir)
Comment on attachment 355840 [details] [diff] [review]
patch v.2

+
+    # mobile optimizations
+    AC_DEFINE(MOZ_GFX_OPTIMIZE_MOBILE)

This isn't really how you should do this. You should just be setting a variable up here that trips the same codepath as after the --enable arg down below. That way there's only one AC_DEFINE of this var, so less chance for things to get out of sync. There are plenty of examples in configure to look at.
Attachment #355840 - Flags: superreview?(ted.mielczarek) → superreview-
Flags: wanted1.9.1?
Attached patch patch v.3 (obsolete) — Splinter Review
right, there was already a OSSO that did the direct AC_DEFINE or whatever.
Attachment #355840 - Attachment is obsolete: true
Attachment #355840 - Flags: review?(vladimir)
Attachment #358929 - Flags: review?(ted.mielczarek)
Comment on attachment 358929 [details] [diff] [review]
patch v.3

+fi
+
+
+
+dnl ========================================================
+dnl enable mobile optimizations

nit: kill a few of those extra empty lines you added.

+AC_SUBST(MOZ_GFX_OPTIMIZE_MOBILE)
+
+MOZ_ARG_ENABLE_BOOL(mobile-optimize,
+[  --enable-mobile-optimize   Enable mobile optimizations],
+    MOZ_GFX_OPTIMIZE_MOBILE=1,
+    MOZ_GFX_OPTIMIZE_MOBILE= )
+if test "$MOZ_GFX_OPTIMIZE_MOBILE"; then
+    AC_DEFINE(MOZ_GFX_OPTIMIZE_MOBILE)
 fi

So, the way you have this written, if you --enable-mobile-optimize, the value will never get AC_SUBSTed. You want to move the AC_SUBST down below the MOZ_ARG_ENABLE_BOOL. You can also get rid of the fourth argument to MOZ_ARG_ENABLE_BOOL, I think.
Attachment #358929 - Flags: review?(ted.mielczarek) → review-
Attached patch patch v.4Splinter Review
Attachment #358929 - Attachment is obsolete: true
Attachment #359348 - Flags: review?(ted.mielczarek)
Attachment #359348 - Flags: review?(ted.mielczarek) → review+
Flags: wanted1.9.1? → wanted1.9.1+
http://hg.mozilla.org/mozilla-central/rev/13a7e1e0134e
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Blocks: C192ConfSync
Flags: in-testsuite-
Target Milestone: --- → mozilla1.9.2a1
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.