While looking for other things under hal/ i noticed the following chunk in hal?Makefile.in, within ifeq (Linux,$(OS_TARGET)) 79679: ifdef MOZ_ENABLE_DBUS 79679: CPPSRCS += UPowerClient.cpp 92976: else 92976: CPPSRCS += FallbackBattery.cpp 79679: endif That's a bit wrong, since upower is not linux only. It also builds and runs fine on OpenBSD and FreeBSD (also probably builds on NetBSD but there's no dedicated backend yet, see http://cgit.freedesktop.org/upower/tree/src). I dont know if MOZ_ENABLE_DBUS can be set on osx (i guess so), so maybe the same ifdef MOZ_ENABLE_DBUS -then-enable-upower chunk should be added to the last else chunk, so that poor other platforms get a chance to talk to upower too ? It doesnt seem to me that this battery api is used for anything else on desktop firefox, but who knows, in the future it might ?

That patch correctly builds UPowerClient.o here if dbus is enabled.

Landry, what about just adding a line for BSD's instead?

Something like that ?

Comment on attachment 627656 [details] [diff] [review] Buld upowerclient on BSDs too if dbus is enabled Review of attachment 627656 [details] [diff] [review]: ----------------------------------------------------------------- r=me but avoid the change at the end if unnecessary. ::: hal/Makefile.in @@ +135,5 @@ > ifeq (gonk,$(MOZ_WIDGET_TOOLKIT)) > # So that we can call nsScreenManagerGonk::GetConfiguration(). > LOCAL_INCLUDES += -I$(topsrcdir)/widget/gonk > LOCAL_INCLUDES += -I$(topsrcdir)/widget/xpwidgets > +endif What are you changing here?

(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #4) > Comment on attachment 627656 [details] [diff] [review] > Buld upowerclient on BSDs too if dbus is enabled > > Review of attachment 627656 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me but avoid the change at the end if unnecessary. > > ::: hal/Makefile.in > @@ +135,5 @@ > > ifeq (gonk,$(MOZ_WIDGET_TOOLKIT)) > > # So that we can call nsScreenManagerGonk::GetConfiguration(). > > LOCAL_INCLUDES += -I$(topsrcdir)/widget/gonk > > LOCAL_INCLUDES += -I$(topsrcdir)/widget/xpwidgets > > +endif > > What are you changing here? Hmm. i guess that's mercurial/vim that added that, dunno if there's a policy wrt newline at EOF in the codebase.. -endif \ No newline at end of file +endif

New patch without that chunk, setting checkin-needed.