Closed Bug 758849 Opened 8 years ago Closed 8 years ago

UPower isnt linux-only

Categories

(Core :: Hardware Abstraction Layer (HAL), defect)

Other
OpenBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: gaston, Assigned: gaston)

References

Details

Attachments

(2 files, 1 obsolete file)

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 ?
Depends on: 696041
That patch correctly builds UPowerClient.o here if dbus is enabled.
Assignee: nobody → landry
Attachment #627453 - Flags: review?(mounir)
Landry, what about just adding a line for BSD's instead?
Something like that ?
Attachment #627453 - Attachment is obsolete: true
Attachment #627453 - Flags: review?(mounir)
Attachment #627656 - Flags: review?(mounir)
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?
Attachment #627656 - Flags: review?(mounir) → review+
(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.
Keywords: checkin-needed
Status: NEW → ASSIGNED
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla15
Attachment #627881 - Flags: checkin+
http://hg.mozilla.org/integration/mozilla-inbound/rev/ce3d4fd7033b
Status: ASSIGNED → NEW
Target Milestone: mozilla15 → ---
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/ce3d4fd7033b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Blocks: 788414
You need to log in before you can comment on or make changes to this bug.